chore: improve css import results; optimize precommit runs #5200

castastrophe · 2025-03-16T16:25:11Z

Description

Pre-commit improvements

Previous the husky pre-commit task was running analysis and linting on files whether they were present in the commit or not. This update adds a specificity to the pre-commit tool by ensuring commands are run only on those files being modified by that commit.

Most relevantly, yarn analyze was running the lit-analyzer on 5,840 files every single commit even if no changes were made to lit-based assets.

CSS import improvements

Prettier's line wrapping for CSS does not have exceptions for var functions; adding whitespace or newlines to var functions cannot be minified safely downstream (and many minifiers won't) because whitespace is valid in a CSS var function. By increasing the line-length for CSS, we prevent invalid newlines being adding into var functions.

This PR adds a spectrum-*.css exception to prettier to allow for up-to-500 characters per line to prevent aggressive line wrapping that will break that downstream minification.

It also swaps stylelint configurations to only lean on the header plugin by default and adds the stylelint-config settings for files not being imported from Spectrum CSS. Spectrum CSS already processes the assets according to rigorous Stylelint configurations and these were stripping important vendor prefixes from the results.

How has this been tested?

Setup: Before testing the husky pre-commit hooks, add exit 1 to the bottom of the pre-commit script. Docs. Remove this once testing is complete and do not commit that change.

To validate the update to line 6 of .husky/pre-commit (eslint), update any *.ts file in the project, git add . && git commit -m "chore: testing pre-commit hook. Expect it to run eslint against that file (but no others).
To validate the update to line 7 of .husky/pre-commit (lit-analyzer), update any packages/*/src/**/*.ts file in the project, git add . && git commit -m "chore: testing pre-commit hook. Expect it to run lit-analyzer against that file (but no others).
To validate the update to line 8 of .husky/pre-commit (linting css), update any packages/**/*.css file in the project, git add . && git commit -m "chore: testing pre-commit hook. Expect it to run stylelint against that file (but no others).
We can already validate the prettier and stylelint updates by reviewing the committed packages/*/src/spectrum-*.css files which now have significantly fewer line breaks inside of var functions.
To validate no regressions in how stylelint formats non-imported CSS assets, make a change to packages/accordion/src/accordion.css to remove the copyright and add -webkit-animation: alternate; to the :host selector. Run yarn lint:css and expect to see:

packages/accordion/src/accordion.css
  1:1  ✖  Header not found                              header/header
  6:5  ✖  Expected empty line before declaration        declaration-empty-line-before
  6:5  ✖  Unexpected vendor-prefix "-webkit-animation"  property-no-vendor-prefix

Types of changes

Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
N/A I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

changeset-bot · 2025-03-16T16:25:14Z

⚠️ No Changeset found

Latest commit: 26eee28

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

castastrophe · 2025-03-16T16:25:47Z

package.json

-        "spectrum-css": "wireit",
-        "spectrum-tokens": "wireit",
-        "spectrum-vars": "wireit",


These commands don't exist in wireit and haven't for a long while - seems safe to remove the alias'es right?

castastrophe · 2025-03-16T16:26:11Z

package.json

-    "peerDependencies": {
-        "common-tags": "^1.8.0"
-    },


This implies consumers checking out the monorepo root would need to install common-tags locally for themselves in order to be able to build this project but I don't believe that is the case. Can we remove this? I don't think the monorepo root has any required peerDeps.

castastrophe · 2025-03-16T16:26:38Z

package.json

@@ -385,7 +379,7 @@
            ]
        },
        "process-spectrum": {
-            "command": "node ./scripts/spectrum-vars.js && node ./tasks/process-spectrum.js && node ./scripts/generate-tokens.js && yarn lint:css --fix && pretty-quick --pattern \"{packages,tools}/**/*.css\" && pretty-quick --pattern \"packages/dialog/src/spectrum-dialog.css\"",
+            "command": "node ./scripts/spectrum-vars.js && node ./tasks/process-spectrum.js && node ./scripts/generate-tokens.js && yarn lint:css --fix && pretty-quick --pattern \"{packages,tools}/**/*.css\"",


I removed the additional pretty-quick run against spectrum-dialog because it's already included in the pattern for the previous pretty-quick run.

github-actions · 2025-03-16T16:27:31Z

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

github-actions · 2025-03-16T16:29:27Z

Tachometer results

Currently, no packages are changed by this PR...

Rajdeepc · 2025-03-17T13:18:29Z

.husky/pre-commit

@@ -1,8 +1,13 @@
 STAGED_FILES_TO_LINT=$(git diff --name-only --cached --diff-filter=d -- "*.ts" "*.js")
+STAGED_FILES_TO_ANALYZE=$(git diff --name-only --cached --diff-filter=d -- "{packages,tools}/*/src/**/!(*.css).ts")


This is nice! :)
Just for my knowledge, is git diff op faster than globbing through packages?

What do you mean by "globbing through packages"? Git diff is the only way, as far as I know, to fetch the list of files changed: https://git-scm.com/docs/git-diff

castastrophe requested a review from a team as a code owner March 16, 2025 16:25

castastrophe commented Mar 16, 2025

View reviewed changes

castastrophe self-assigned this Mar 16, 2025

castastrophe added dependencies Tooling CSS Processing ready-for-review labels Mar 16, 2025

Rajdeepc reviewed Mar 17, 2025

View reviewed changes

castastrophe force-pushed the chore-improve-imports-optimize-precommit branch 6 times, most recently from 4324c96 to fa86913 Compare March 25, 2025 14:20

chore: improve css import results; optimize precommit runs

26eee28

castastrophe force-pushed the chore-improve-imports-optimize-precommit branch from fa86913 to 26eee28 Compare March 25, 2025 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: improve css import results; optimize precommit runs #5200

chore: improve css import results; optimize precommit runs #5200

castastrophe commented Mar 16, 2025 •

edited

Loading

changeset-bot bot commented Mar 16, 2025 •

edited

Loading

castastrophe Mar 16, 2025

castastrophe Mar 16, 2025

castastrophe Mar 16, 2025

github-actions bot commented Mar 16, 2025 •

edited

Loading

github-actions bot commented Mar 16, 2025

Rajdeepc Mar 17, 2025 •

edited

Loading

castastrophe Mar 18, 2025

		@@ -1,8 +1,13 @@
		STAGED_FILES_TO_LINT=$(git diff --name-only --cached --diff-filter=d -- ".ts" ".js")
		STAGED_FILES_TO_ANALYZE=$(git diff --name-only --cached --diff-filter=d -- "{packages,tools}//src//!(.css).ts")

chore: improve css import results; optimize precommit runs #5200

Are you sure you want to change the base?

chore: improve css import results; optimize precommit runs #5200

Conversation

castastrophe commented Mar 16, 2025 • edited Loading

Description

Pre-commit improvements

CSS import improvements

How has this been tested?

Types of changes

Checklist

Best practices

changeset-bot bot commented Mar 16, 2025 • edited Loading

⚠️ No Changeset found

castastrophe Mar 16, 2025

Choose a reason for hiding this comment

castastrophe Mar 16, 2025

Choose a reason for hiding this comment

castastrophe Mar 16, 2025

Choose a reason for hiding this comment

github-actions bot commented Mar 16, 2025 • edited Loading

Branch preview

Review the following VRT differences

github-actions bot commented Mar 16, 2025

Tachometer results

Rajdeepc Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

castastrophe Mar 18, 2025

Choose a reason for hiding this comment

castastrophe commented Mar 16, 2025 •

edited

Loading

changeset-bot bot commented Mar 16, 2025 •

edited

Loading

github-actions bot commented Mar 16, 2025 •

edited

Loading

Rajdeepc Mar 17, 2025 •

edited

Loading