Skip to content

fix(site-import): keep a repeated class rule at its later cascade position - #573

Open
tommy230 wants to merge 1 commit into
CoreBunch:mainfrom
tommy230:fix/site-import-duplicate-class-order
Open

tommy230 wants to merge 1 commit into
CoreBunch:mainfrom
tommy230:fix/site-import-duplicate-class-order

Conversation

@tommy230

@tommy230 tommy230 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A stylesheet that styles the same bare class twice, with other rules in between, imports with the wrong winner. Frameworks do this on purpose: a grid sheet sets a column's gutter early, then zeroes it hundreds of rules later so a wrapper can carry it instead. After import the early value came back.

.x { padding-left: 15px }
.y { padding-left: 7px }
.x { padding-left: 0 }

In a browser an element with class="x y" gets padding-left: 0: the last equal-specificity rule wins. After import it gets 7px. processBaseSelector in cssToStyleRules.ts merged the second .x into the first, so the merged rule sat before .y and .y won the tie.

The fix:

  • Every occurrence of a bare class selector becomes its own rule at its own source position. The duplicate-class warning still fires per repeat.
  • Repeats stay kind: 'class' through parsing, so cross-sheet conflict detection still merges the file's full definition in order. normalizeBindableClassRules demotes them to ambient fragments after resolution, which is how repeats across files already work per docs/features/site-import.md.
  • A class whose @media or @supports block comes before its first base rule still imports as one class rule: the base declarations fill the rule the block opened. main also raised a spurious duplicate-class warning for that shape; it no longer does. An authored empty .x {} is not filled, so a later .x keeps its own position.

Trade-off: the class panel shows the first occurrence's declarations; a later repeat's override lives in an ambient fragment at its own position. That matches the existing behavior for the same class defined in two files. A @media block that follows a repeat now attaches to that latest occurrence, so the published cascade is right but the class panel no longer lists that breakpoint override.

Out of scope: a page that links the same stylesheet twice (the second link is still deduplicated), and @media overrides that attach to the base rule's position rather than the block's.

Verification

  • bun run build
  • bun run lint
  • bun test: 7057 pass, 0 fail
  • Docker/deployment check, if relevant: not relevant

Across the two touched test files (74 tests), 6 fail on main and pass with this change, all of them new or rewritten cases in src/__tests__/siteImport/cssToStyleRules.test.ts. src/__tests__/siteImport/duplicateClassConflicts.test.ts passes on both and guards conflict detection against the new rule shape.

Checklist

  • Tests cover behavior changes.
  • Docs were updated when behavior, config, deployment, or public surfaces changed. (docs/features/site-import.md.)
  • No compatibility shim was added for old pre-release behavior.
  • No secrets, local databases, uploads, or generated artifacts are included.

🤖 Generated with Claude Code

@tommy230
tommy230 force-pushed the fix/site-import-duplicate-class-order branch from 310db0d to b8f5e8f Compare September 28, 2026 20:10
…ition

A stylesheet that styled the same bare class twice had its second rule
merged into the first, so the merged rule sat before any rule written
between the two occurrences and lost cascade ties it wins in a browser.
Each occurrence is now its own rule at its own source position. Both
stay class-kind through parsing so cross-sheet conflict detection still
sees the file's full definition; the registry demotes repeats at commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tommy230
tommy230 force-pushed the fix/site-import-duplicate-class-order branch from b8f5e8f to f45cd30 Compare September 29, 2026 00:01
@tommy230
tommy230 marked this pull request as ready for review September 29, 2026 04:47

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant