-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Revert "[fix] add filename to combined source map if needed (#6089)" #6572
Conversation
This reverts commit 4ca2af4.
the og. PR contained more commits, should they also be reverted? |
There's only a single commit after being squashed. It's a bit weird because the PR being reverted (#6089) and #6427 both fixed the same issue in different ways and so both updated the same set of tests. So this PR doesn't revert most of the tests since the test updates were also merged as part of #6427 |
for the record, this PR in vite: vitejs/vite#2904 (merged but unreleased as of typing) might fix it too. |
Ahh! That would explain then why all the reports are coming in just for SvelteKit. They said they're doing a new release of Vite tomorrow, so maybe we should hold off on this PR then and see what the behavior is with the new version of Vite. It sounds like it may print a warning then which we can investigate to see what the issue is |
vite 2.4.4 prints a warning instead of the ENOENT error. |
I still think we should move ahead with this PR to fix the warnings people are getting with SvelteKit. Vite folks said the source maps being added here were wrong - they should be relative to output file and not relative to project root: vitejs/vite#4423 (comment). There's not as strong a need for this change since #6427 as committed |
here's the spec https://sourcemaps.info/spec.html#h.75yo6yoyk7x5 this could be fixed without reverting by setting a relative source then? either by passing in a different filename from vite-plugin-svelte or stripping the path before setting source. |
Yes, it could be fixed by setting a relative source. I'm not sure it's a priority though. I could definitely find higher ROI things to work on |
This seems to be a problem within compile itself, the change for preprocess just uncovered it:
I think this can be solved by calculating the correct relative path (just the filename minus path unless outputFilename is set and a different basepath) once and then using that in all 3 places mentioned above. |
This reverts commit 4ca2af4.
Fixes sveltejs/kit#2029