-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat(turbopack): Enable lightningcss for turbopack by default #7524
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
7 Ignored Deployments
|
🟢 Turbopack Benchmark CI successful 🟢Thanks |
Logs
See job summary for details |
|
{"offset": {"line": 28, "column": 1}, "map": {"version":3,"sources":[],"names":[],"mappings":"A"}}, | ||
{"offset": {"line": 34, "column": 0}, "map": {"version":3,"sources":["/turbopack/[project]/crates/turbopack-tests/tests/snapshot/css/chained-attributes/input/style.css"],"sourcesContent":["@import url(\"./a.css\") layer(layer) supports(not(display: inline-grid)) print;\n\n.style {\n color: yellow;\n}\n"],"names":[],"mappings":"AAEA,CAAC,KAAK,CAAC,CAAC;EACN,KAAK,EAAE,MAAM;AACf,CAAC"}}, | ||
{"offset": {"line": 36, "column": 1}, "map": {"version":3,"sources":[],"names":[],"mappings":"A"}}] | ||
{"offset": {"line": 4, "column": 0}, "map": {"version":3,"sources":["[project]/crates/turbopack-tests/tests/snapshot/css/chained-attributes/input/c.css"],"names":[],"mappings":"A"}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sourcesContent
is missing in SourceMaps and the mappings
are empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried debugging with dbg
, but it seems like the source map produced by lightningcss
is relatively minimal at the first place.
cc @devongovett Can you take a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sourcesContent, you need to call source_map.set_source_content
for each source index to add it. Lightning CSS doesn't store a reference or copy to the original code so that's up to you. I see here where you add the source files but I couldn't find where the source content gets set so maybe that's missing? Or you could get the content some other way here.
As for the mappings themselves, I'm not sure without debugging it myself, but I see a couple possibilities:
- Maybe try adding the sources before calling
to_css
instead of after? - When converting the source map to your internal format, maybe check that the source index is passed through (right now it is always
None
). Looks like maybe you can useadd_raw
instead. Not sure if that is related. At least verify that there are mappings there.turbo/crates/turbopack-css/src/process.rs
Lines 837 to 844 in b61c98e
builder.add( m.generated_line, m.generated_column, m.original.map(|v| v.original_line).unwrap_or_default(), m.original.map(|v| v.original_column).unwrap_or_default(), None, None, );
The mappings are probably more sparse than you might expect though. Basically there's one per rule since that's the level of granularity that browser dev tools operate at, so more is unnecessary. But it shouldn't be empty at least.
...g/output/crates_turbopack-tests_tests_snapshot_css_css-legacy-nesting_input_style_7d7e1c.css
Outdated
Show resolved
Hide resolved
Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
# Turbopack * vercel/turbo#7682 <!-- Will Binns-Smith - Turbopack HMR: url-encode sourceURLs --> * vercel/turbo#7524 <!-- Donny/강동윤 - feat(turbopack): Enable lightningcss for turbopack by default --> ### What? Enable lightningcss by default, for `--turbo` mode. ### Why? It's time to do it 😄 ### How? Turbopack counterpart: vercel/turbo#7524 Closes PACK-2600
❤️ |
Description
Enable lightningcss by default, for --turbo mode.
Testing Instructions
next.js counterpart: vercel/next.js#62565
Closes PACK-2601