Skip to content
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

Use lightningcss to implement CSS packager #8492

Merged
merged 12 commits into from Jan 8, 2024
Merged

Use lightningcss to implement CSS packager #8492

merged 12 commits into from Jan 8, 2024

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Sep 20, 2022

Fixes #8162

This re-implements the CSS packager using the bundleAsync function of lightningcss. This means there is support for bundling @import rules with cascade layers and supports conditions, and improved support for media queries. The logic for bundling these is fairly complicated, and can't really be done fully correctly without an AST.

Need to do some testing on real world apps, and some perf testing.

@parcel-benchmark
Copy link

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.43s -41.00ms
Cached 348.00ms -3.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

React HackerNews 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js ✅

Timings

Description Time Difference
Cold 6.89s +46.00ms
Cached 295.00ms +4.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

# Conflicts:
#	packages/optimizers/css/package.json
#	packages/packagers/css/package.json
#	packages/packagers/css/src/CSSPackager.js
#	packages/transformers/css/package.json
#	packages/transformers/css/src/CSSTransformer.js
#	yarn.lock
contents += `@import "${url}";\n`;
lineOffset++;
}
let res = await bundleAsync({
Copy link
Member

Choose a reason for hiding this comment

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

What can we do for the REPL where bundleAsync doesn't exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does exist now (as of a few months ago)! Uses emscripten's asyncify transform to turn the rust function into a coroutine.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, perfect!

Copy link
Member

@mischnic mischnic Jan 7, 2024

Choose a reason for hiding this comment

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

We will need to usual alias (and devdep) to the wasm package though like in the CSS packager and optimizer:

"browser": {
"lightningcss": "lightningcss-wasm"
}

Copy link
Member Author

Choose a reason for hiding this comment

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

added the dep. are we just relying on the transformer to initialize the wasm module? looks like CSSOptimizer doesn't do it, so I just followed that.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, I apparently forgot that in the optimizer. It's not really safe to assume that every worker running the packager also has also at least once invoked the CSS transformer.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok added to both

@devongovett devongovett merged commit cd447dc into v2 Jan 8, 2024
15 of 16 checks passed
@devongovett devongovett deleted the lightningcss branch January 8, 2024 03:38
lettertwo added a commit that referenced this pull request Jan 30, 2024
* upstream/v2: (22 commits)
  Add source map support to the inline-require optimizer (#9511)
  [Web Extension] Add content script world property to manifest schema validation (#9510)
  feat: add getCurrentPackageManager (#9505)
  Default Bundler Contributor Notes (#9488)
  rename parentAsset to root for msb config and remove unstable (#9486)
  Macro errors -> v2 (#9501)
  Statically evaluate constants referenced by macros (#9487)
  Multiple css bundles in Entry bundle groups issue (#9023)
  Fix macro issues (#9485)
  Bump follow-redirects from 1.14.7 to 1.15.4 (#9475)
  Revert more CI changes to centos job (#9472)
  Use lightningcss to implement CSS packager (#8492)
  Fixup CI again (#9471)
  Clippy and use napi's Either3 (#9047)
  Upgrade to eslint 8 (#8580)
  Add support for JS macros (#9299)
  Fixup REPL CI (#9467)
  Drop per-pipeline transformation cache (#9459)
  Upgrade some CI actions (#9466)
  REPL (#9365)
  ...
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.

CSS @import layers are ignored in some situations
3 participants