fix: avoid null sourcePath in server.sourcemapIgnoreList
#12251
Conversation
|
|
| path.isAbsolute(sourcePath) && | ||
| path.isAbsolute(mod.file) | ||
| ) { | ||
| if (path.isAbsolute(sourcePath) && path.isAbsolute(mod.file)) { | ||
| map.sources[sourcesIndex] = path.relative( |
There was a problem hiding this comment.
@danielroe was it discussed if we should call sourcemapIgnoreList using the sourcePath after 303? I don't know what is best here, but maybe it isn't a bad idea to get the path that will end up showing in the dev tools?
There was a problem hiding this comment.
I think @bmeurer and I felt it would be better to call it with an absolute path: #12174 (comment).
There was a problem hiding this comment.
Thanks for the reference, I missed that one. @bmeurer what is the reason you consider safer to go with absolute paths? My first reaction while reading those messages was of suggesting to align as close as possible with rollup (we still have time to change this if we think it is better).
There was a problem hiding this comment.
rollup passes project relative paths. I guess that would be fine here as well, but I'm not sure how to reconstruct the project relative paths here.
There was a problem hiding this comment.
I checked with a fresh create-vite Vue Typescript app, and the paths I currently get are:
sourcemapIgnoreList: (sourcePath, sourcemapPath) => boolean
> pnpm run build
sourcePath: ../../node_modules/.pnpm/@vue+shared@3.2.47/node_modules/@vue/shared/dist/shared.esm-bundler.js
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../node_modules/.pnpm/@vue+reactivity@3.2.47/node_modules/@vue/reactivity/dist/reactivity.esm-bundler.js
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../node_modules/.pnpm/@vue+runtime-core@3.2.47/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../node_modules/.pnpm/@vue+runtime-dom@3.2.47/node_modules/@vue/runtime-dom/dist/runtime-dom.esm-bundler.js
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../../../../../vite.svg
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../src/assets/vue.svg
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../src/components/HelloWorld.vue
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
sourcePath: ../../src/main.ts
sourcemapPath: /Users/patak/test/sourcemap-test/dist/assets/index-!~{001}~.js.map
So it looks like the sourcePath in rollup is relative to the sourcemapPath, no?
For dev we get
> pnpm run dev
sourcePath: overlay.ts
sourcemapPath: /Users/patak/vite-work/packages/vite/dist/client/client.mjs.map
sourcePath: client.ts
sourcemapPath: /Users/patak/vite-work/packages/vite/dist/client/client.mjs.map
sourcePath: /Users/patak/test/sourcemap-test/src/main.ts
sourcemapPath: /Users/patak/test/sourcemap-test/src/main.ts.map
sourcePath: /Users/patak/test/sourcemap-test/src/App.vue
sourcemapPath: /Users/patak/test/sourcemap-test/src/App.vue.map
sourcePath: env.ts
sourcemapPath: /Users/patak/vite-work/packages/vite/dist/client/env.mjs.map
sourcePath: /Users/patak/test/sourcemap-test/src/components/HelloWorld.vue
sourcemapPath: /Users/patak/test/sourcemap-test/src/components/HelloWorld.vue.map
The ones we care about in this discussion are the .vue files and the main.ts. So, if we want to align with rollup, I think sourcePath would end up being main.ts, App.vue, and HelloWorld.vue. So the file relative to .map path of sourcemapPath. This means that users should mainly be using sourcemapPath to do the condition instead of sourcePath during dev. The absolute path seems easier to work with as a user. Maybe the alignment isn't that important here. cc @bluwy @sapphi-red for a second opinion.
There was a problem hiding this comment.
I think it's worth aligning with Rollup here to avoid the confusion that it works in dev, and later not in builds. We could rename the first param as relativeSourcePath like Rollup so it's consistent.
patak-dev
added
the
p3-minor-bug
Description
Add missing guard against
nullsourcePathafter:sourcemapIgnoreListconfiguration option #12174Fixes https://github.com/vitejs/vite-ecosystem-ci/actions/runs/4305319550/jobs/7507588927
cc @danielroe @bmeurer
What is the purpose of this pull request?