-
Notifications
You must be signed in to change notification settings - Fork 438
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 absolute urls for published sourcemap urls #7599
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Component Testing Report Updated Oct 7, 2024 3:54 PM (UTC) ✅ All Tests Passed -- expand for details
|
⚡️ Editor Performance ReportUpdated Mon, 07 Oct 2024 16:05:08 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
6c151a1
to
7251d1d
Compare
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.
This is great for debugging, thank you!
Tested it locally and matches the expectation
Description
The source maps we upload to our module CDN now contains source maps, which helps in debugging. Sources are inlined, but the original source locations are also left intact. This isn't a problem per se, but it does lead to tooling trying to link to them and failing. For instance, a source map could be referencing
../../../node_modules/.pnpm/@sanity+client@6.22.0_debug@4.3.7/node_modules/@sanity/client/dist/index.browser.js
, which would be resolved tohttps://modules.sanity-cdn.com/modules/v1/node_modules/.pnpm/@sanity+client@6.22.0_debug@4.3.7/node_modules/@sanity/client/dist/index.browser.js
, which we obviously do not host.It is helpful to have some source location, as from the example above I can see that the source pointed to
@sanity/client
. To make this even more helpful, this PR takes the approach of rewriting these sources to point to absolute URLs.Technically speaking this is "incorrect", as we don't use the URLs given for actually building the bundles, however it is useful for debugging purposes, and leaving the original paths do not make sense as we are not hosting the source files on the CDN anyway.
The URLs we rewrite to are as follows:
node_modules
), we usejsdelivr.net
.raw.githubusercontent.com
instead, but it is less user-friendly for us developers.What to review
Testing
I've built the bundles locally and ran a modified uploading script (disabling the actual upload logic) and verified that the sources gets correctly rewritten.
Notes for release
None