Skip to content

fix: Stop repeating the license header in every compiled stylesheet - #4906

Draft
TrevorBurnham wants to merge 1 commit into
cloudscape-design:mainfrom
TrevorBurnham:fix-license-headers-in-partials
Draft

fix: Stop repeating the license header in every compiled stylesheet#4906
TrevorBurnham wants to merge 1 commit into
cloudscape-design:mainfrom
TrevorBurnham:fix-license-headers-in-partials

Conversation

@TrevorBurnham

Copy link
Copy Markdown
Contributor

Description

Fixes #4905.

Sass copies loud comments into the compiled CSS, so each partial's license header was emitted once per entry point that @uses it. Every published stylesheet carried a median of 35 copies, and 598 KB of the 2326 KB of CSS was the same four lines repeated.

The 89 partials under src/ now declare the header with Sass line comments, which the compiler strips. The styles.scss entry points keep the block comment, so every output file still carries one. .stylelintrc enforces the right form for each using the commentType option added in cloudscape-design/build-tools#75.

Published CSS drops 2326 KB → 1770 KB (-23.9%). Gzipped it is 306 KB → 300 KB, because gzip already deduped the identical blocks; the win is in unminified bytes, which is what dev servers serve. Minified production bundles are unaffected either way, since minifiers strip the comments already.

Two things reviewers should know:

1. Every generated scoped class name changes. The name is awsui_{name}_{fileHash}_{contentHash}_{lineNumber}, and removing comments moves both the content hash and the line numbers. The test-utils selectors snapshot passes unchanged because it truncates to the path-derived hash. The design tokens snapshot is updated in this PR because tokenStylesSuffix hashes src/internal/styles/global.scss, whose header changed. The compiled CSS is otherwise identical: I diffed all 229 stylesheets before and after, normalising license blocks and generated hashes, and the payload matches everywhere.

2. This does not remove every duplicate. The median goes from 35 to 2, not to 1. The remainder comes from two sources outside this change:

  • @cloudscape-design/component-toolkit ships focus-visible/index.scss and style-api/index.scss with block headers. Converting those locally saves a further 17 KB and takes the median to 1.
  • Dart Sass re-emits an entry point's leading comments once per module when the entry point and its partials share a dependency. Minimal repro on Sass 1.89.2, no Cloudscape code involved:
// _shared.scss
.shared { color: gray; }
@mixin m { color: gray; }

// _a.scss     -> @use "shared";  .a { color: red; }
// _b.scss     -> @use "shared";  .b { color: blue; }

// entry.scss
/* ENTRY HEADER */
@use "shared";
@use "a";
@use "b";

ENTRY HEADER is emitted three times. Drop the @use "shared" from the entry, or from _a/_b, and it is emitted once. This is the shape of every Cloudscape component that has sub-partials, which is why popover still ends up with 5 headers rather than 1.

Both are worth follow-ups; neither blocks this change.

How has this been tested?

  • npm run lint:stylelint passes with build-tools#75 applied. It fails with 89 Missing license header errors against the currently published build-tools, so this PR is blocked on feat: Add commentType option to the license-headers stylelint rule build-tools#75 merging. That is why it is a draft.
  • npm run quick-build, then a byte-level comparison of all 229 compiled stylesheets before and after, as described above.
  • npx jest src/popover src/table src/test-utils src/internal/styles — 736 tests pass.
  • src/__tests__/snapshot-tests/test-utils-selectors.test.tsx passes with no snapshot change, confirming the public test-utils selectors are unaffected.
  • design-tokens.test.ts snapshot updated; the diff is only the tokenStylesSuffix hash.
  • documenter.test.ts fails locally on main too, because it needs lib/components-definitions from the full build rather than quick-build. Unrelated.
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Sass copies loud comments into the compiled CSS, so each partial's header
was emitted once per entry point that @uses it. Every published
stylesheet carried a median of 35 copies, and 598 KB of the 2326 KB of
CSS was the same four lines repeated.

Partials now declare the header with Sass line comments, which the
compiler strips. Entry points keep the block comment, so every output
file still carries one. Stylelint enforces the right form for each via
the new commentType option.

Published CSS drops from 2326 KB to 1770 KB. The remaining duplicates
come from two sources outside this repo, both tracked in the issue:
@cloudscape-design/component-toolkit's SCSS, and a Sass behaviour that
re-emits an entry point's leading comments once per module when the
entry and its partials share a dependency.

The compiled CSS is otherwise byte-identical. Generated scoped class
names change because the name embeds a content hash and a line number,
and the design tokens snapshot changes because tokenStylesSuffix hashes
internal/styles/global.scss.

Requires cloudscape-design/build-tools#75.
Refs cloudscape-design#4905.
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.

[Bug]: Published CSS contains 30-50 duplicate license headers per file (26% of CSS bytes)

1 participant