feat(source-maps): Add debugIds option#342
Conversation
🦋 Changeset detectedLatest commit: 783a15d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
colinaaa
left a comment
There was a problem hiding this comment.
This looks GREAT to me! I’ve added a few comments to better align it with Rspack.
|
Thank you for the feedback. It makes sense to me as well to align with webpack impl. I'll also add the tests. |
Signed-off-by: Qingyu Wang <40660121+colinaaa@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
Bundle ReportBundle size has no change ✅ |
CodSpeed Performance ReportMerging #342 will not alter performanceComparing Summary
|
colinaaa
left a comment
There was a problem hiding this comment.
LGTM! Thanks
Looks like the CI is failing. You need to:
- Format the code with
pnpm dprint fmt - Update API report with
pnpm turbo api-extractor -- --local. See: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md#update-api-references
|
@colinaaa Thank you for the help. ✅ formatted the code and updated the API. |
Head branch was pushed to by a user without write access
…onfigurations and use cspell.jsonc
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/web-constants@0.10.0 ### Minor Changes - feat: rewrite the main thread Element PAPIs ([#343](#343)) In this commit we've rewritten the main thread apis. The most highlighted change is that - Before this commit we send events directly to bts - After this change, we send events to mts then send them to bts with some data combined. ### Patch Changes - feat: The onNapiModulesCall function of lynx-view provides the fourth parameter: `lynxView`, which is the actual lynx-view DOM. ([#350](#350)) - fix: publicComponentEvent args order ([#401](#401)) - Updated dependencies \[[`a521759`](a521759)]: - @lynx-js/web-worker-rpc@0.10.0 ## @lynx-js/web-core@0.10.0 ### Minor Changes - feat: rewrite the main thread Element PAPIs ([#343](#343)) In this commit we've rewritten the main thread apis. The most highlighted change is that - Before this commit we send events directly to bts - After this change, we send events to mts then send them to bts with some data combined. ### Patch Changes - refactor: timing system ([#378](#378)) Now we moved the timing system to the background thread. - feat: support `defaultOverflowVisible` config ([#406](#406)) - fix(web): rsbuild will bundle 2 exactly same chunk for two same `new Worker` stmt ([#372](#372)) the bundle size will be optimized about 28.2KB - fix: inline style will be removed for value number `0` ([#368](#368)) the inline style value could be incorrectly removed for number value `0`; For example, `flex-shrink:0` may be ignored. - feat: The onNapiModulesCall function of lynx-view provides the fourth parameter: `lynxView`, which is the actual lynx-view DOM. ([#350](#350)) - fix: publicComponentEvent args order ([#401](#401)) - Updated dependencies \[[`3a8dabd`](3a8dabd), [`a521759`](a521759), [`890c6c5`](890c6c5)]: - @lynx-js/web-worker-runtime@0.10.0 - @lynx-js/web-constants@0.10.0 - @lynx-js/web-worker-rpc@0.10.0 ## @lynx-js/web-mainthread-apis@0.10.0 ### Minor Changes - feat: rewrite the main thread Element PAPIs ([#343](#343)) In this commit we've rewritten the main thread apis. The most highlighted change is that - Before this commit we send events directly to bts - After this change, we send events to mts then send them to bts with some data combined. ### Patch Changes - feat(web): use pure DOM API to implement Element PAPIs ([#334](#334)) 1. rewrite all element PAPIs impl. Now we use DOM. 2. use our new package `@lynx-js/offscreen-document` to support the new Element PAPI implementation in a worker - fix: inline style will be removed for value number `0` ([#368](#368)) the inline style value could be incorrectly removed for number value `0`; For example, `flex-shrink:0` may be ignored. - fix: publicComponentEvent args order ([#401](#401)) - Updated dependencies \[[`3a8dabd`](3a8dabd), [`a521759`](a521759), [`890c6c5`](890c6c5)]: - @lynx-js/web-constants@0.10.0 ## @lynx-js/web-worker-runtime@0.10.0 ### Minor Changes - feat: rewrite the main thread Element PAPIs ([#343](#343)) In this commit we've rewritten the main thread apis. The most highlighted change is that - Before this commit we send events directly to bts - After this change, we send events to mts then send them to bts with some data combined. ### Patch Changes - Updated dependencies \[[`2a8ddf3`](2a8ddf3), [`3a8dabd`](3a8dabd), [`878050a`](878050a), [`a521759`](a521759), [`890c6c5`](890c6c5)]: - @lynx-js/web-mainthread-apis@0.10.0 - @lynx-js/web-constants@0.10.0 - @lynx-js/web-worker-rpc@0.10.0 ## @lynx-js/react@0.106.2 ### Patch Changes - fix: prevent multiple firstScreen events when reloading before `jsReady` ([#377](#377)) - Optimize the bundle size by eliminating unnecessary code when the lazy bundle is not utilized. ([#284](#284)) ## @lynx-js/rspeedy@0.8.7 ### Patch Changes - Support using `-debugids` in `output.sourceMap.js`. ([#342](#342)) See [Source Map Debug ID Proposal](https://github.com/tc39/ecma426/blob/main/proposals/debug-id.md) for more details. - Use `chunkLoading: 'import-scripts'` for Web platform ([#352](#352)) - Support `output.distPath.*`. ([#366](#366)) See [Rsbuild - distPath](https://rsbuild.dev/config/output/dist-path) for all available options. - Support `performance.printFileSize` ([#336](#336)) Whether to print the file sizes after production build. ## @lynx-js/react-rsbuild-plugin@0.9.4 ### Patch Changes - feat: add extractStr option to pluginReactLynx ([#391](#391)) - Convert background-only files from js to ts ([#346](#346)) - Updated dependencies \[[`f849117`](f849117), [`d730101`](d730101), [`42217c2`](42217c2), [`f03bd4a`](f03bd4a)]: - @lynx-js/react-webpack-plugin@0.6.10 - @lynx-js/template-webpack-plugin@0.6.7 - @lynx-js/react-alias-rsbuild-plugin@0.9.4 - @lynx-js/react-refresh-webpack-plugin@0.3.2 - @lynx-js/css-extract-webpack-plugin@0.5.2 - @lynx-js/web-webpack-plugin@0.6.3 ## @lynx-js/web-elements@0.5.2 ### Patch Changes - fix: When list with list-type: flow, scrolltoupper, scrolltolower were specified, there was a blank block. ([#379](#379)) - fix: do not show scroll bar ([#406](#406)) ## @lynx-js/web-worker-rpc@0.10.0 ### Patch Changes - feat: The onNapiModulesCall function of lynx-view provides the fourth parameter: `lynxView`, which is the actual lynx-view DOM. ([#350](#350)) ## @lynx-js/react-webpack-plugin@0.6.10 ### Patch Changes - feat: add extractStr option to pluginReactLynx ([#391](#391)) - Fix issue with lazy loading of bundles when source maps are enabled. ([#380](#380)) - Fix issue where loading a lazy bundle fails if it does not return a webpack chunk. ([#365](#365)) ## @lynx-js/template-webpack-plugin@0.6.7 ### Patch Changes - fix: merge different chunk groups for same output filename ([#371](#371)) ## create-rspeedy@0.8.7 ## @lynx-js/react-alias-rsbuild-plugin@0.9.4 ## upgrade-rspeedy@0.8.7 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
I've noticed that
rspeedyis using SourceMapDevToolPlugin to generate source maps, but it doesn't expose thedebugIdsoption. This option is helpful when debugging production errors as it links correct source map to the correct bundle. More at https://github.com/tc39/ecma426/blob/main/proposals/debug-id.mdI've tried the change locally with a new Lynx project and the debugIds comments are generated as expected above existing
sourcemappingurl.I'm curious if you would be open to add this option and what needs to be done to do so.
Checklist