Repository navigation
Fixes #36189: Read Tailwind at-rules in the UI import guard - #36654
Merged
Merged
Conversation
The guard read relative specifiers in style files only after @import and url(. A relative path in Tailwind's @source, @reference, @plugin or @config, or in Less's data-uri(), could leave src/ and still pass, though it resolves to nothing in Collate's copy of the UI. The guard now reads all of them. The existing @source in src/styles/tailwind.css stays inside src/ and passes. Both decision records now name the two Collate copy steps: copy-om-to-collate copies src/** except three test-setup files, and copy-om-playwright-to-collate copies playwright/**. The rule is unchanged. The new record's list of forms the guard does not read now matches the widened pattern. Each record gets a revision entry and an amendment.
harshach
added
the
safe to test
Contributor
Jest test CoverageUI tests summary
|
The new record now lists every style form the guard does not read, including `@source not`, upper-case function names, a `data-uri()` whose first argument is not quoted, and paths built from Less escapes or variables. It also stops saying that every path leaving `src/` resolves to nothing in Collate, because Collate copies `playwright/` into the same folder. The query console record's v2 amendment is back to its landed wording. Records are append-only, and the v3 amendment already corrects it.
The record's list of style forms the guard does not read now names Less's image-size(), image-width() and image-height() and Tailwind's source(...) option, and ends with any other function or option that takes a path, so it no longer reads as complete. It also notes that the guard accepts a path that climbs above the UI folder and comes back into src/. Such a path resolves in OpenMetadata and to nothing in Collate.
Code Review ✅ Approved🟡 Medium risk · Expanding the UI import-boundary regex changes which style references fail tests and could block builds if it misparses valid paths. Extends the UI import boundary guard to read relative specifiers in Tailwind at-rules ( VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
9 of 10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36189
Follow-up to #36642, closing two minors from its second review.
What the guard missed.
uiImportBoundary.test.tsread relative specifiers in.lessand.cssfiles only after@importandurl(. A relative path in Tailwind's@source,@reference,@pluginor@config, or in Less'sdata-uri(), could leavesrc/and still pass. Collate copies onlysrc/, so such a path resolves to nothing there.src/styles/tailwind.cssalready uses@sourceand@plugin.What it reads now.
STYLE_SPECIFIERalso reads those four at-rules anddata-uri(...), including the two-argumentdata-uri('mime', '../x')form. The existing@source '../**/*.{tsx,ts,jsx,js,css}'intailwind.cssis now read and resolves insidesrc/.Record correction. Both decision records now name both Collate copy steps.
copy-om-to-collatecopiessrc/**except three test-setup files, andcopy-om-playwright-to-collatecopies the end-to-end tests inplaywright/**into the same folder. Production code never imports end-to-end tests, so the rule is unchanged. The new record lists the forms the guard still does not read, such as an upper-case at-rule,@source not, a path built with a Less escape or variable,image-size()and Tailwind'ssource(...)option. It also names a path that climbs above the UI folder and comes back intosrc/, which resolves in OpenMetadata and to nothing in Collate. A reviewer checks those forms. Each record gets a revision entry and an amendment.Red and green runs. Each line was appended temporarily and then restored. Each fails the guard at this head and passes it with the
mainpattern.mainpatterntailwind.css:@source '../../../openmetadata-ui-core-components/src/**/*.tsx';tailwind.css:@reference '../../public/x.css';tailwind.css:@plugin '../../vite/plugin.js';tailwind.css:@config '../../tailwind.config.js';KnowledgeCenterWidget.less:data-uri('../../../../public/x.png')KnowledgeCenterWidget.less:data-uri('image/svg+xml;charset=UTF-8', '../../../../public/x.svg')At this head, the guard passes (
Tests: 1 passed).Other checks.
uiImportBoundary,OntologyStudio.constants,OntologyStudio.utilsandOntologyStudioQueryConsole: 4 suites, 42 tests passed.tsc --noEmiterror set is identical tomain's (35 pre-existing errors, none in a changed file).--max-warnings=0andprettier --checkpass on the changed file.check_decision_records.pypasses atdf16d79ffc5.🤖 Generated with Claude Code