-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[fix] use consistent relative source filename for js sourcemaps #6598
[fix] use consistent relative source filename for js sourcemaps #6598
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.
lgtm. I'm not sure if there's an easy way to add a test for this?
there is https://github.com/sveltejs/svelte/tree/master/test/sourcemaps and a new sample could be added, but it requires some rethinking / extra work on index.ts too because currently outputFilename is hardcoded and the existing samples never use filenames with paths. |
…ut filename is specified
turns out you can override it in sample config, so i've added a sample that fails without the changes in Component.ts and passes with them. Now that i spent more time looking at this i wonder if a missing outputFilename in compilerOptions is an error in itself or if it should fall back to filename instead 🤔 |
I'm seeing this error too.
I'm using adapter-static |
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.
LGTM
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.
probably include some tests for different combinations of compile_options.filename
, compile_options.outputFilename
and compile_options.cssOutputFilename
?
not really, this is just pretty-printing file paths ... but yepp @tanhauhau |
it's actually fixing the pointer in the source map to the source file. Vite is complaining that Svelte's source maps are broken because that file path is relative to the project root and not the source map. there's some more details about that in the links provided in the PR description |
basic combinations for filename + outputFilename are covered by I have added a css preprocessor to |
Before submitting the PR, please make sure you do the following
[feat]
,[fix]
,[chore]
, or[docs]
.Tests
npm test
and lint the project withnpm run lint
sveltekit issue for vite printing warnings about wrong sourcemap paths: sveltejs/kit#2029
and vite issue here: vitejs/vite#4423 (comment)
also this PR/discussion if a change should be reverted that could also fix it: #6572
note while i did test that with this change vite no longer logs that warning in kit, i'm not 100% sure that this fix is 100% correct. I'm confused why there were 3 different ways to handle it before and it may very well have been on purpose.
cc @dummdidumm @tanhauhau