Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(vite): allow overriding vite sourcemap #7342

Merged
merged 3 commits into from
Sep 8, 2022

Conversation

danielroe
Copy link
Member

πŸ”— Linked issue

resolves nuxt/nuxt#14840

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Since we started setting sourcemap per-client, per-server, we are overriding the user's choice if they have configured this manually, which they may wish to do.

This respects the value of vite.build.sourcemap if set.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe danielroe added bug Something isn't working vite πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage labels Sep 8, 2022
@danielroe danielroe requested a review from pi0 September 8, 2022 08:10
@danielroe danielroe self-assigned this Sep 8, 2022
@netlify
Copy link

netlify bot commented Sep 8, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit 6af2c34
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/6319b0d446f78300085455bf

@pi0
Copy link
Member

pi0 commented Sep 8, 2022

This might be a breaking change but having multiple sources of trust is probably more dangerous to introduce such cases. Any reason we can't rely on one top level sourcemap and flag vite option as never with replacement same as #7317?

Copy link
Member Author

I think issue is that vite allows multiple kinds of sourcemaps. e.g. 'hidden' or 'inline'.

@pi0
Copy link
Member

pi0 commented Sep 8, 2022

I see. It is same for webpack too actually. I was going to suggest in initial refactor to extend boolean to other string values per type. But in this case, for now it can be enabled with top level sourcemap.client (it is a breaking change) and keep using vite config until we can do that. I don't think implicitly auto enabling client sourcemap was right behavior.

Copy link
Member Author

I'm not sure I understand what you are suggesting. What do you mean by "implicitly auto enabling client sourcemap"? What are you saying the problem is with the previous PR?

@pi0
Copy link
Member

pi0 commented Sep 8, 2022

No i mean changes with previous PR was right (we just forgot to mark it as breaking change). Before PR, setting vite.build.sourcemap was setting + enabling (new) for both client and server. Now we have dedicated flags for each environment. With changes of this PR, setting vite.build.sourcemap keeps enabling sourcemap even if sourcemap.client is set to false. And we might rely on it's value in other places.

@@ -51,7 +51,7 @@ export async function buildClient (ctx: ViteBuildContext) {
dedupe: ['vue']
},
build: {
sourcemap: ctx.nuxt.options.sourcemap.client,
sourcemap: ctx.config.build?.sourcemap ?? ctx.nuxt.options.sourcemap.client,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be reversed. options.sourcemap.client? config.build.sourcemap : false this way we both respect enabling flags and vite configuration for type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution!

@pi0 pi0 merged commit 2b37d72 into main Sep 8, 2022
@pi0 pi0 deleted the fix/allow-overriding-vite-sourcemap branch September 8, 2022 13:52
This was referenced Sep 9, 2022
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x bug Something isn't working πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage vite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NuxtConfig: vite.build.sourcemap = "hidden" won't output source map files.
2 participants