Repository navigation
UN-4299 [FEAT] v1 design tokens, themes and style isolation (P0-T04) - #2399
Merged
Merged
Conversation
…N-4270) Adds frontend/src/v1/ as an isolated tree for the v1 frontend (P0-T01): - Folder taxonomy from the spec, a public entry (src/v1/index.ts, imported by v0 as `@v1`) and a cloud overlay placeholder. - src/v1/tsconfig.json: strict, no JS, unused locals/params are errors, `@v1` alias only. v0's tsconfig/jsconfig exclude src/v1. - Biome overrides for the two-way import boundary and v1 bans (no `../`, `@/`, prop-types, antd, component CSS; noExplicitAny). - scripts/check-v1.mjs for what Biome cannot express: TypeScript-only files, the folder taxonomy and import contract, PropTypes, `ant-`. - typecheck:v1, lint:v1 and test:v1 scripts; v0 scripts are unchanged. - test-fixtures/v1-boundary/ proves each rule fails and passes as specified, run with node --test so the v0 vitest suite never sees it. - .gitignore re-includes src/v1/lib/ (the Python `lib/` rule matched it). v0 build output, test counts, typecheck and Biome results are unchanged.
…270) check-v1.mjs: - Scan comments with a string/template-aware tokenizer; a `/*` or `//` inside a string or line comment no longer hides the imports after it. - Read template-literal specifiers, import.meta.glob and `import(..., opts)`. - Flag specifiers that leave src/v1 (`/`, `..`, `@/`, `src/`) itself, not only via Biome, which skips template literals. - Check `@import`/`url()` and `ant-` in styles/ CSS; report symlinks; catch built `ant-` names; treat .test/.spec .mts/.cts as test files; ignore .DS_Store; drop the unused --root option. biome.json: also ban `..`, `./../**`, `/**`, `antd/**`, `prop-types/**`. tsconfig: skipLibCheck off so v1's own .d.ts files are checked. .gitignore: re-include lib/ at any depth under src/v1. Fixtures: a generated matrix over every folder-contract pair, every import form, every outside spelling, a symlink case, and cases for each new rule; tsc cases assert error codes; output matching excludes the command text and spawn errors fail loudly. Comment and README corrections.
… (UN-4270) - ci-frontend-lint: the typecheck job also runs `typecheck:v1` and `lint:v1`, so the strict v1 tsconfig and the scripts/check-v1.mjs checks (folder contract, TypeScript-only files, PropTypes, `ant-`) gate PRs now rather than from P0-T10. - Pages may no longer import app/, test/ or the v1 entry, only the shared folders (components, shell, services, stores, hooks, lib, styles, plugins). Test files in a page may still import test/. Contract table, README and the fixture matrix updated together.
…(UN-4270) check-v1.mjs: split the comment scanner into small helpers (cognitive complexity 40 -> under 15), drop the nested ternary and nested template literal, use String.raw for the import regexes, keep CLI flags in a Set. Fixture runner: - Cases and the expected folder contract are parsed text tables instead of repeated object literals (the 10% duplication). - Sorts use an explicit localeCompare comparator. - The contract matrix is built by a helper. - Spawns /bin/sh by absolute path and no longer needs `git init` (Biome only needs the ignore file). The git check-ignore call keeps a PATH lookup, with the reason and a NOSONAR marker. Fixture cases: re-exports rewritten as small functions and read-only props, without changing what each case exercises.
…nnot see (UN-4270) Addresses Greptile review on #2380: - import.meta.glob([...]): every pattern in the array is checked, not only the first, so an allowed first pattern no longer hides a forbidden one. - check-v1 --code also scans v0 (src/ outside v1, including the cloud src/plugins overlay) for template-literal imports, globs, require(), @/v1, src/v1, relative paths and CSS @import that reach into v1 other than through `@v1`. Biome still covers plain imports. Fixtures: Greptile's example as a case, a glob array with a forbidden later pattern, and a generated test of every v0 -> v1 form (denied and allowed). The v0 scan reports nothing on the current tree.
…i 5.2 both declare containSubset Co-Authored-By: Claude Opus 5.5 <[email protected]>
… per-browser preference Co-Authored-By: Claude Opus 5.5 <[email protected]>
…tore Co-Authored-By: Claude Opus 5.5 <[email protected]>
…in three modes Co-Authored-By: Claude Opus 5.5 <[email protected]>
… match the v1 Tailwind root Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…t it Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ercase letters only Co-Authored-By: Claude Opus 5.5 <[email protected]>
…heme mapping and the focus utility Co-Authored-By: Claude Opus 5.5 <[email protected]>
…so biome ci passes src/v1/styles/tokens.css is written by scripts/build-v1-tokens.mjs and checked byte-for-byte by styles/tokens.test.ts; the formatter would rewrap it and break the check. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…sts and on missing variants Co-Authored-By: Claude Opus 5.5 <[email protected]>
…and portal sync Co-Authored-By: Claude Opus 5.5 <[email protected]>
…tEffect Co-Authored-By: Claude Opus 5.5 <[email protected]>
…line styles, CSS imports and !important fail lint Co-Authored-By: Claude Opus 5.5 <[email protected]>
… arbitrary values Co-Authored-By: Claude Opus 5.5 <[email protected]>
… and the fixtures v0's root auto-detected sources across the whole frontend, so the v1 tree, scripts/check-v1-styles.mjs and test-fixtures/ added 12 unprefixed utilities and 8 theme variables to v0's stylesheet. With these three @source not lines the v0 build is byte-identical to the base again (239/239 files). Co-Authored-By: Claude Opus 5.5 <[email protected]>
…longer cancel each other Co-Authored-By: Claude Opus 5.5 <[email protected]>
…our arbitrary values and !important after a variant Co-Authored-By: Claude Opus 5.5 <[email protected]>
…cope variants, shadows and the root fill to v1 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…the theme test exercises the mode effect Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ge, the pixel escape and the current unprefixed-class rationale Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
|
Re-checked on
Please let me know once it's ready for re-review. Generated by Claude Code |
…fold has landed The scaffold PR #2380 was squash-merged, so every scaffold file both sides touched conflicted. Main's version is taken for the import checker, the tsconfigs, the CI workflow and the Biome boundary groups; this branch keeps the styling policy script and its lint:v1 and tokens:v1 entries, the tokens.css formatter override, the Styling section of the v1 README and the 27 styling fixture rows. Two fixtures main re-added or changed carried an unscoped :root selector with a raw hex, which the styling policy rejects, so they reference a token under .v1 instead. Co-Authored-By: Claude Fable 5.1 <[email protected]>
… from review
S3 let a unit followed by `_` through (`ui:grid-cols-[200px_1fr]`,
`ui:m-[2rem_auto]`): the lookahead after the unit rejected `\w`, which
includes Tailwind's in-bracket separator. It now rejects letters and `-`
only, and fixture v1-raw-pixel-arbitrary-separator pins both forms.
S8 read every literal inside cva() as a class list, so a normal
`defaultVariants: { variant: "outline" }` failed because outline is also a
utility name. classSiteLiterals now tracks the property chain inside cva()
and treats as classes only the base string, the values under variants and
a compound `class`/`className`; a property name, a defaultVariants value or
a compoundVariants matcher is a choice, not a class. The passing fixture
carries a full cva config; v1-cva-unprefixed-variant proves a variant
value is still checked.
S6 reported every nested `& + x` as leaving the root, but under `.v1 .menu`
it resolves to `.v1 .menu + x`, inside v1. selectorPreludes now records
whether every enclosing rule is one compound on the root, and only then
does a `&` sibling leave. The nested fixture is rewritten to the leaving
forms (directly under `.v1`, and under `&[data-theme="dark"]`), and the
passing fixture gains the safe ones.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
Author
|
All three are on the branch at c6c0b21: |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
muhammad-ali-e
approved these changes
Nov 2, 2026
muhammad-ali-e
left a comment
muhammad-ali-e
left a comment
Contributor
There was a problem hiding this comment.
Re-checked on c6c0b21c7: all six review items and the three follow-ups are fixed, each confirmed with a probe file (S3 [200px_1fr]/[2rem_auto], cva() variant names, nested & + inside v1, prefix order, named colours, vh/vw), and typecheck:v1, lint:v1 and all 89 fixture rows pass. Approving. Thanks, Praveen!
Generated by Claude Code
ritwik-g
reviewed
Nov 2, 2026
…narCloud findings
SonarCloud's duplication check ignores literal values, so the 75 same-shaped
entries in styles/tokens.ts read as copies of each other (91% of the file,
16.8% of the PR's new lines, gate threshold 3%). The values now live in
styles/tokens.json, one array per family in board order; tokens.ts keeps the
Token type and builds TOKENS from it, deriving cssVar from name (true for
every entry) and expanding a single string to all three modes. tokens.css is
regenerated (header only) and the lint no longer needs a raw-values exemption
for tokens.ts.
Also, from Ritwik's pass over the remaining findings:
- CSS_UNIT_RE: `\d+(?:\.\d*)?|\.\d+` matches the same strings without the
overlapping alternation (javascript:S8786).
- The stem escape `s.replace("-", "\\-")` was a no-op: `-` is literal outside
a character class.
- The file-type dispatch uses `endsWith(".css")` (javascript:S6557).
- `.sonarcloud.properties` excludes frontend/test-fixtures/**: the boundary
fixtures break the v1 rules on purpose.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
athul-rs
added a commit
that referenced
this pull request
Nov 3, 2026
… and the scaffold README: the tokens branch's newer wording throughout. package.json: its bun runner for tokens:v1, keeping this stack's plugin fixtures in test:v1. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
Deepak-Kesavan
added a commit
that referenced
this pull request
Nov 4, 2026
…2399 landed The theme slot in src/v1/test/providers.tsx swaps its stand-in for app/theme/ThemeProvider, the one-file change the harness was built for. prepareProviders stores the requested theme for it to hydrate and creates #v1-portal with lib/portal's getPortalContainer; resetProviders clears the portal, the theme storage key and the store after each test. PORTAL_CONTAINER_ID, V1_ROOT_CLASS and ThemeMode now come from lib/.
Deepak-Kesavan
added a commit
that referenced
this pull request
Nov 5, 2026
…rig group (#2429) * UN-4329 [FEAT] Add the v1 unit-test harness and the frontend-v1-unit rig group - vitest.v1.config.mts runs src/v1 (minus plugins/) with its own setup, coverage report (coverage/v1) and 80% threshold; vitest.v1-plugins.config.mts runs cloud v1 plugins. vitest.config.mjs now excludes src/v1. - src/v1/test: renderV1/renderHookV1 over one pluggable provider chain (QueryClient, MemoryRouter, v1 theme root + #v1-portal), typed session and distribution, MSW at the HTTP boundary with a catch-all that fails the test naming every unmocked request, axe-core assertions, observer/matchMedia fakes. - Rig: optional vitest_config on a group (vitest runner only, must exist inside workdir), passed as --config; existing groups' commands are unchanged. - test-fixtures/v1-harness proves an unmocked request fails a real vitest run. - Deps (exact): msw 2.15.0, axe-core 4.14.0, @vitest/coverage-v8 3.2.6, @tanstack/react-query 5.104.5. * UN-4329 [FEAT] Keep v0's transforms out of the v1 config and document the harness v0's optionalPluginImports() stubbed a missing import under any /plugins/ path, so a missing v1 module imported as {} instead of failing (UN-4286's missing-import test caught it). The v1 config now declares its own plugins (react, svgr) and only the @v1 alias. * UN-4329 [FIX] Close the harness's unmocked-request gaps found in self-review - WebSocket: a catch-all ws.link closes unmocked sockets (1011) and records the URL; before, MSW opened a mocked socket and the test passed. - Requests started in cleanup() (unmount effects) now fail the test that unmounted: afterEach awaits pending interceptors before checking. - Requests after a file's last test fail the file from afterAll. - server.resetHandlers(...args) keeps the catch-alls last. - Overlapping axe runs are queued instead of throwing 'Axe is already running'. - A test leaving fake timers on gets real timers back before teardown. Each gap has a known-bad case in test-fixtures/v1-harness. * UN-4329 [FIX] Let mocked WebSockets through, reconnect observer fakes, export ws - The WebSocket catch-all steps aside when a test's own ws.link() matches the URL: MSW runs every matching ws handler, so it closed mocked sockets too. - Observer fakes re-register on observe() after disconnect(). - @v1/test re-exports ws. - The harness fixtures run the lockfile's vitest via process.execPath instead of resolving npx from PATH. * UN-4329 [FEAT] Render v1 tests inside the real ThemeProvider now that #2399 landed The theme slot in src/v1/test/providers.tsx swaps its stand-in for app/theme/ThemeProvider, the one-file change the harness was built for. prepareProviders stores the requested theme for it to hydrate and creates #v1-portal with lib/portal's getPortalContainer; resetProviders clears the portal, the theme storage key and the store after each test. PORTAL_CONTAINER_ID, V1_ROOT_CLASS and ThemeMode now come from lib/. * UN-4329 [MISC] Clear the two Sonar issues on the harness - setup.ts: settle() awaits two macrotask turns without an await-in-loop. - test_groups.py: build the manifest outside pytest.raises so only load_groups() can raise. * UN-4329 [FIX] Drop renderV1's session and distribution options until a provider reads them No ProviderSlot and no v1 code outside test/ consumed context.session or context.distribution, so session: null, makeSession({ role: "user" }) and distribution: "onprem" rendered the same tree as the defaults, and a test of a role or distribution branch passed without reaching it. A test-only context would not fix that: v1 components may not import test/. renderV1/renderHookV1 no longer take either (type error), and throw if one is passed anyway. TestSession/makeSession stay as fixture data for apiPath. Each option returns with the slot that reads it (README table: session store; plugin registry, UN-4285/UN-4286). New test: each accepted option (theme) changes what renders. * UN-4329 [FIX] Report a vitest group that ran no tests as empty, not a truncated junit vitest under passWithNoTests writes <testsuites tests="0"> with no <testsuite> child. parse_junit found no counters and reported Errors 1, so an empty frontend-v1-unit-cloud (no cloud v1 plugin yet) went red wherever Node exists. parse_junit now reads a childless <testsuites> root's counters, and a vitest group that exits 0 having run nothing is recorded as exit 5, the rig's "nothing collected" code, so it reads empty rather than a green row that verified nothing. Exit 5 never fails a non-pytest group. * UN-4329 [FIX] Measure v1 coverage on whole-suite runs only The coverage threshold is global, so a focused run (one test file) failed it whatever its tests did: vitest run --config vitest.v1.config.mts src/v1/lib/cn.test.ts passed 4/4, then exited 1 at 3.34% lines. Coverage is now on when CI is set (the rig and every workflow) and in bun run test:v1 (--coverage); a focused local run skips it. Requested by Ritwik in the approving review.
3 tasks done
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.
What
src/v1/styles/tokens.json(one array per family, board order),styles/tokens.tstypes them and derives each CSS variable from the token name, andbun run tokens:v1generatessrc/v1/styles/tokens.css, scoped to.v1[data-theme="…"]. Values come from the Figma "Midnight Bloom" variable collection and the dev-spec board (node 683:135); the table the P1-T01 gallery will render isrenderTokenTable(TOKENS)and its snapshot is committed.src/v1/styles/index.css, compiled withprefix(ui)andsource(none)+@source "../": every v1 utility isui:…, every theme variable--ui-…, only v1 files are scanned, and preflight is not imported a second time. Theme keys are bound to the runtime tokens through@theme inline; the stock colour, radius, shadow, text and font scales are reset so only token-backed utilities exist.dark:andunstract:variants key ondata-theme; one focus treatment,ui:focus-ring, implements the board's rule.lib/theme.ts,stores/theme.ts, preference keyunstract.v1.theme, follow-system; each mode block also sets the browser'scolor-scheme, so native controls and scrollbars followdata-theme), a portal container#v1-portal.v1for dialogs, menus and toasts (lib/portal.ts),cn()configured for the prefix (lib/cn.ts), andThemeProvider(app/theme/ThemeProvider.tsx), which renders the v1 root (data-ui-version="v1", the marker the E2E harness in UN-4345 [FEAT] Frontend v1 E2E harness and the frontend-v1-e2e group #2355 expects), hydrates the stored mode before first paint and keeps the portal in step.scripts/check-v1-styles.mjstolint:v1: raw colours (hex, functions, palette and named, in code and CSS), raw lengths (anywhere inside an arbitrary value, and px/rem/em/pt/vh/vw in CSS), inlinestyle,!important, CSS imports outsideapp/andstyles/, selectors outside.v1(a sibling combinator right after the root included, at top level or nested directly in the root;.v1 .menu { & + x }stays inside),prefers-color-schemein v1 CSS, and utilities withoutui:first (hover:ui:flexcompiles to nothing) all fail lint.classNamevalues and the class positions ofcn()/cva()are always checked (incva()the base string, variant values and a compoundclass; a variant name or choice such asdefaultVariants: { variant: "outline" }is not a class); other strings when they look like a class list. Twenty-nine guardrail fixtures intest-fixtures/v1-boundary/pin each rule, three passing fixtures pin what the rules must allow. README gains a Styling section.@fontsourcepackages v0 already pins: Inter 400/500/600 (the only weights the type roles use) and Geist Mono 400.Why
.text-sm) with different theme values and overwrite each other in the sharedutilitieslayer. A prefix makes the two sets disjoint by construction; v1 also survives v0's deletion in P9.How
ui, notv1v1is rejected at compile time. Raised in #engg-unstract-frontend-v1-migration on 28 Oct.baselayer would re-runborder: 0 solidand turn v0's borders black. v1 ships scoped base rules inbase.css; P9 adds preflight whensrc/index.cssgoes.--background,--radius-md), scoped under.v1[data-theme]:root/.dark. Tailwind's own variables live in the separate--ui-namespace.@source notlines in v0'ssrc/index.cssskipLibCheck: trueinsrc/v1/tsconfig.json@types/chai^5.2 both declarecontainSubset; with library checks on, the first v1 test file fails TS2300, and no@types/chaiversion in vitest's accepted range avoids it. Turn back off once vitest no longer clashes. Scaffold change; Jaseem and Harini, please note.tokens.cssbiome ci src/would otherwise rewrap it.variant="outline",type="hidden"and UI copy; shape detection still catches a constant holding unprefixed classes.styleattribute that sets only CSS custom properties (style={{ "--page-aspect": w / h }})ui:classes read (ui:aspect-(--page-aspect)); the classes keep every visual decision. A string, a variable, a spread or a plain key still fails, so the keys stay visible at the call site.lib/css-custom-properties.d.tsaugments React'sCSSPropertieswith--*keys, which strict TS otherwise rejects (TS2353); the passing fixture is typechecked to exercise it. Raised by Ali on 29 Oct.ThemeProvideruses layout effects--spacingis--space-1, named steps 1 to 8 are their tokensui:p-5used Tailwind's stock scale and--space-5drove nothing (Greptile). The bare multiplier stays soui:w-64andui:size-4keep working;ui:p-5is nowvar(--space-5).tokens.json, not in the TypeScript sourcetokens.tscounted as copies (16.8% of new lines against a 3% gate). JSON is not checked for duplication, and the file is the data anyway;cssVarwas--+namefor every entry, so it is derived. Ritwik's preferred option.tokens:v1runs under bunregisterHooksresolver for that, which exists only from 22.15 while the engines field allows 20.19 (Greptile). bun imports them directly.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
@v1yet, so none of the new code or CSS is in the production bundle: the v0 build output is byte-identical to the base commit (239 files, SHA-256 compared; seeclaudedocs/frontend-v1-lane-c/p0-t04/no-regression.mdin the lane folder). The v0 vitest count is unchanged (629).src/index.css, and only to stop scanning three non-v0 folders; its output is identical.@v1, the v1 stylesheet joins the bundle. Measured delta on a scratch build: 363 added rules, 0 removed; every added rule is.v1- orui\:-scoped, a--ui-/--tw-variable, an@font-faceidentical to one v0 already declares, or a@layerstatement, plus one duplicate block of Tailwind's--tw-*defaults with values identical to v0's. No v0 rule changes.Database Migrations
Env Config
Relevant Docs
frontend/src/v1/README.md, Styling section.Related Issues or PRs
mainon 2 Nov;mainis merged into this branch (not rebased) so the five P1-T21 viewer PRs stacked on it keep clean diffs.unstract.v1.theme; P0-T03 wraps the v1 tree inThemeProvider; P1-T01's gallery readsTOKENS; a v1components.jsonshould settailwind.prefixtoui.destructive-muted(board section 8) andsidebar-primary/sidebar-ring(spec text). To be added when the designer defines them.Dependencies Versions
@tailwindcss/node4.3.3 added as a dev dependency (same version as@tailwindcss/vite), used only by the root compile test.Notes on Testing
.sonarcloud.propertiesexcludesfrontend/test-fixtures/**from analysis: the boundary fixtures break the v1 rules on purpose.cn, root compile, provider) plus 89 guardrail fixture rows (bun run test:v1, 29 of them for the styling policy).typecheck:v1,lint:v1andbiome ci src/clean (Biome's 798 warnings are v0's, unchanged).index.csswith@tailwindcss/nodeand asserts: onlyui:-prefixed utilities, every emitted selector scoped (checked per comma-separated part, with an in-test negative proof), variants keyed on.v1[data-theme], the focus ring's forced-colors outline, no!important, noprefers-color-scheme, no stock palette or scales, and every custom property either a--ui-/--tw-variable or a declared token.cn()extends tailwind-merge'stextscale with the eight type roles, soui:text-body ui:text-foregroundkeeps both (tailwind-merge would otherwise read the role as a colour and drop it); a test ties that list to thetypefamily inTOKENS.src/index.css, so no layered v1 rule can beat them, and v1 lint bans!important): the global scrollbar colours, the Radix popperz-index: 1500 !important, the sonner toaster rule, and thesvg.lucidesizing rule (its:not(.flex, .inline-flex, .grid)does not exemptui:inline-flex). All harmless today; listed in the v1 README and in the evidence file so nobody is surprised.#v1-portal). The byte-identical build and the classified CSS delta stand in for the first today.🤖 Generated with Claude Code