Skip to content

chore: use zimmerframe for CSS analysis/transformation #10482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Feb 15, 2024
Merged

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Feb 14, 2024

Right now, CSS handling lives in its own universe — we parse the <style> tag (sloppily), then bludgeon it into shape with classes defined in Stylesheet.js and Selector.js that both use zimmerframe and reimplement its traversal behaviour. This takes more code and more CPU cycles than just using the same techniques we use everywhere else for analysis and traversal.

As I started working on :is(...) support I realised it's also a fairly inflexible approach. I therefore intend to remove a lot of the code in Stylesheet.js and Selector.js in favour of something more harmonious. Right now it looks like more code, but I'm reasonably confident it will end up being significantly less once it's done.

No changeset because this is all internal stuff

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Feb 14, 2024

⚠️ No Changeset found

Latest commit: 0e7fc47

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

@Rich-Harris
Copy link
Member Author

(note: the diff is more green than red, but that's because of the tests — the AST now contains more granular detail, which means there's a bunch of bigger output.json files)

Copy link
Contributor

@trueadm trueadm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rich-Harris Rich-Harris merged commit cec3540 into main Feb 15, 2024
@Rich-Harris Rich-Harris deleted the css-zimmerframe branch February 15, 2024 20:27
@AlbertMarashi
Copy link

@Rich-Harris can you link relevant PRs for the CSS nesting you might create so I can follow along

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.

4 participants