-
Notifications
You must be signed in to change notification settings - Fork 28.5k
fix(turbopack): keep the original sourcemap of styles after source transform #79700
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
base: canary
Are you sure you want to change the base?
fix(turbopack): keep the original sourcemap of styles after source transform #79700
Conversation
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
40fc7e3
to
9d5acfb
Compare
What do you think about propagating the errors? Because the webpack loaders are user-provided, probably the best solution here is to assume they're not guaranteed to be correct and emit some sort of non-fatal diagnostic or issue collectible of some sort, falling back to But I also think it's close enough to just assume webpack loaders emit correct source maps unless we find evidence to the contrary.
|
Failing test suitesCommit: b2372e3
Expand output● app dir - basepath › should successfully hard navigate from pages -> app
Read more about building and testing Next.js in contributing.md.
Expand output● socket-io › should support socket.io without falling back to polling
Read more about building and testing Next.js in contributing.md.
Expand output● app dir - rsc basics › next internal shared context › should not error if just load next/navigation module in pages/api
● app dir - rsc basics › next internal shared context › should not error if just load next/router module in app page
● app dir - rsc basics › should correctly render page returning null
● app dir - rsc basics › should correctly render component returning null
● app dir - rsc basics › should correctly render layout returning null
● app dir - rsc basics › should correctly render page returning undefined
● app dir - rsc basics › should correctly render component returning undefined
● app dir - rsc basics › should correctly render layout returning undefined
● app dir - rsc basics › should handle named client components imported as page
● app dir - rsc basics › should handle client components imported as namespace
● app dir - rsc basics › should render server components correctly
● app dir - rsc basics › should reuse the inline flight response without sending extra requests
● app dir - rsc basics › should support multi-level server component imports
● app dir - rsc basics › should create client reference successfully for all file conventions
● app dir - rsc basics › should be able to navigate between rsc routes
● app dir - rsc basics › should handle streaming server components correctly
● app dir - rsc basics › should track client components in dynamic imports
● app dir - rsc basics › client references with TLA (node) › should support TLA in sync client reference imports
● app dir - rsc basics › client references with TLA (node) › should support TLA in lazy client reference
● app dir - rsc basics › client references with TLA (edge) › should support TLA in sync client reference imports
● app dir - rsc basics › client references with TLA (edge) › should support TLA in lazy client reference
● app dir - rsc basics › should support next/link in server components
● app dir - rsc basics › should link correctly with next/link without mpa navigation to the page
● app dir - rsc basics › should escape streaming data correctly
● app dir - rsc basics › should render built-in 404 page for missing route if pagesDir is not presented
● app dir - rsc basics › should suspense next/legacy/image in server components
● app dir - rsc basics › should suspense next/image in server components
● app dir - rsc basics › should handle various kinds of exports correctly
● app dir - rsc basics › should support native modules in server component
● app dir - rsc basics › should resolve different kinds of components correctly
● app dir - rsc basics › should stick to the url without trailing /page suffix
● app dir - rsc basics › should support streaming for flight response
● app dir - rsc basics › should support partial hydration with inlined server data
● app dir - rsc basics › should not apply rsc syntax checks in pages/api
● app dir - rsc basics › should not use bundled react for pages with app
● app dir - rsc basics › should use canary react for app
● app dir - rsc basics › should be able to call legacy react-dom/server APIs in client components
● app dir - rsc basics › should support partial hydration with inlined server data in browser
● app dir - rsc basics › should generate edge SSR manifests for Node.js
Read more about building and testing Next.js in contributing.md.
Expand output● app-dir action handling › should forward action request to a worker that contains the action handler (node)
Read more about building and testing Next.js in contributing.md. |
@bgw I thought about propagating the errors before, choosing to fallback to None because that the soucemap correctness may not be a fatal point, it's for user experience. With falling back to None when error occurred in webpack loaders, the sourcemap generated by lightningcss will be used still. |
9d5acfb
to
e607852
Compare
e607852
to
ea60c2a
Compare
CodSpeed Performance ReportMerging #79700 will not alter performanceComparing Summary
Benchmarks breakdown
|
Snapshots updated , and removed redundant clone and add some comments. |
Sorry, change my mind, propagating the errors is better, which can help webpack loader's maintainer being aware of sourcemap corruption. |
105ba66
to
3d57b72
Compare
Why
The original sourcemap be lost after source transform. For example, a
xyz.sass
file processed by sass-loader, return axyz.sass.css
, the output css chunk only contains the sourcemap ofxyz.sass.css
produced by lightningcss,xyz.sass
associated sourcemap been lost.How
Extend lightningcss generated sourcemap with the original sourcemap, see:
https://github.com/parcel-bundler/lightningcss/blob/f2dc67c4d3fe92f26693c02366db1e60cae0db27/napi/src/lib.rs#L776
Related issue
umijs/mako#1915