-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix(nuxt): add sourcemap path transformation for client builds #32313
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
Conversation
|
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.
Pull Request Overview
This PR ensures client build source maps point to the final output directory by transforming their paths.
- Added
sourcemapPathTransform
to adjust source map paths from the temp build directory to.output/public
. - Imported
dirname
andrelative
frompathe
to compute the correct relative paths. - Used
ctx.config.root!
andctx.nuxt.options.app.buildAssetsDir
to locate the public assets directory.
Comments suppressed due to low confidence (3)
packages/vite/src/client.ts:181
- [nitpick] The variable name
newRelativePath
is generic; consider renaming tooutputRelativePath
to clarify that this path targets the final output directory.
const newRelativePath = relative(
packages/vite/src/client.ts:175
- Add unit tests covering
sourcemapPathTransform
for both relative and non-relative source paths to ensure correct behavior across different build scenarios.
sourcemapPathTransform (relativeSourcePath, sourcemapPath) {
packages/vite/src/client.ts:175
- [nitpick] Include a JSDoc comment above
sourcemapPathTransform
describing its purpose, parameters, and return value to aid future maintainers.
sourcemapPathTransform (relativeSourcePath, sourcemapPath) {
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new function, Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vite/src/client.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (1)
packages/vite/src/client.ts (1)
2-2
: LGTM! Import statement correctly updated.The import statement appropriately adds
dirname
andrelative
functions from thepathe
module, which are used in the new sourcemap transformation logic below.
CodSpeed Performance ReportMerging #32313 will not alter performanceComparing Summary
Footnotes |
Is there anything I can do to move this PR forward? Please let me know. |
🔗 Linked issue
Resolves #32009
📚 Description
This change fixes the source map paths in client builds, to point them to the right sourcemaps.
My first contribution to this repo, pls check if I used the
nuxt
config dirs properly.Question: should I add a test for checking the generated sourcemap content? I have seen that all fixtures have disabled sourcemaps, so maybe I need to add a new fixture? LMK!