feat: support custom source map implemetation - #1289
Conversation
4555289 to
f20d60e
Compare
|
Sounds like a good idea. I'll look more in depth soon. |
|
@devongovett Should we maintain rust API compatibility across the 1.0.0 beta releases? |
|
no don't worry about that. |
This reverts commit ca480a6.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59cfeb7caf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dest.write_char('\n')?; | ||
| } | ||
| dest.newline()?; |
There was a problem hiding this comment.
When writing a hoisted nested rule fails, either through the destination writer or a custom at-rule's ToCss implementation, these ? operators return directly from to_css_base before line 332 restores parent. The printer is therefore left holding a raw pointer to the stack-local ctx; reusing it after the recoverable error can dereference a dangling pointer during selector serialization. Run this block inside a closure or use a guard so the context is restored on every error path.
Useful? React with 👍 / 👎.
| let content = sm.get_source_content(orig.source).unwrap().to_owned(); | ||
| map.set_source_content(source_index, &content); |
There was a problem hiding this comment.
When an inline input source map is present, add_mapping now clones and sets the complete original source content for every generated mapping. The previous implementation guarded this operation so it happened only when the source was newly added; with a large Sass/Less source and thousands of mappings, the new path performs roughly mapping_count × source_size copying and can cause extreme allocation and runtime growth. Track which output source indices have already received content and set each only once.
Useful? React with 👍 / 👎.
| original.source = source_index; | ||
| original.name = name; | ||
|
|
||
| let content = sm.get_source_content(orig.source).unwrap().to_owned(); |
There was a problem hiding this comment.
For a custom SourceMap whose from_data_url accepts a map without embedded source content, find_closest_mapping can validly return a location while get_source_content returns None, as allowed by the new trait's Option return type. This unwrap then panics while serializing otherwise valid mapped CSS. Skip setting the content when it is absent rather than requiring every custom input map to contain it.
Useful? React with 👍 / 👎.
2 participants
Summary
This refactors CSS printing so
ToCssdepends on a sealed printer trait instead of the concretePrinter<W, S>type. It also splits source map writing behindcustom_sourcemap, while keeping the existingsourcemapfeature backed by Parcel source maps for current users.Why
Bundlers such as Rspack already have their own source map representation. Previously, they had to generate a Parcel source map from Lightning CSS output and then translate it back into their own source map type.
With this change, those consumers can provide a custom source map writer directly.
This has two benefits:
In Rspack CI, this reduced the binary by about 400 KB:
https://github.com/web-infra-dev/rspack/actions/runs/28227945651/job/83627257783?pr=14581
Release builds are estimated to save around 300 KB.
Compatibility
The default
sourcemapfeature still uses Parcel source maps, so existing Lightning CSS users should keep the same public behavior.Consumers that want to avoid the built-in Parcel implementation can enable
custom_sourcemapand provide their own writer.Validation
Checked the library with and without custom source maps, including no-default-feature and serde-only configurations.