-
Notifications
You must be signed in to change notification settings - Fork 27.8k
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
Create valid URLs in source map sources with Turbopack #76075
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
sources
40a65f2
to
66896c8
Compare
Tests Passed |
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
buildDuration | 19.1s | 16.8s | N/A |
buildDurationCached | 15.9s | 13.2s | N/A |
nodeModulesSize | 393 MB | 393 MB | N/A |
nextStartRea..uration (ms) | 443ms | 436ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
5306-HASH.js gzip | 55.2 kB | 55.2 kB | N/A |
7048.HASH.js gzip | 168 B | 168 B | ✓ |
8377-HASH.js gzip | 5.46 kB | 5.46 kB | N/A |
bccd1874-HASH.js gzip | 53 kB | 53 kB | ✓ |
framework-HASH.js gzip | 57.5 kB | 57.5 kB | N/A |
main-app-HASH.js gzip | 245 B | 245 B | ✓ |
main-HASH.js gzip | 34.9 kB | 34.9 kB | N/A |
webpack-HASH.js gzip | 1.71 kB | 1.71 kB | ✓ |
Overall change | 55.1 kB | 55.1 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 39.4 kB | 39.4 kB | ✓ |
Overall change | 39.4 kB | 39.4 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
_app-HASH.js gzip | 194 B | 194 B | ✓ |
_error-HASH.js gzip | 193 B | 192 B | N/A |
amp-HASH.js gzip | 513 B | 511 B | N/A |
css-HASH.js gzip | 342 B | 342 B | ✓ |
dynamic-HASH.js gzip | 1.84 kB | 1.84 kB | N/A |
edge-ssr-HASH.js gzip | 265 B | 264 B | N/A |
head-HASH.js gzip | 363 B | 360 B | N/A |
hooks-HASH.js gzip | 393 B | 390 B | N/A |
image-HASH.js gzip | 4.59 kB | 4.59 kB | N/A |
index-HASH.js gzip | 268 B | 266 B | N/A |
link-HASH.js gzip | 2.35 kB | 2.35 kB | ✓ |
routerDirect..HASH.js gzip | 327 B | 326 B | N/A |
script-HASH.js gzip | 397 B | 397 B | ✓ |
withRouter-HASH.js gzip | 325 B | 325 B | ✓ |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 3.72 kB | 3.72 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
_buildManifest.js gzip | 749 B | 747 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
index.html gzip | 523 B | 521 B | N/A |
link.html gzip | 539 B | 535 B | N/A |
withRouter.html gzip | 520 B | 517 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
edge-ssr.js gzip | 130 kB | 130 kB | N/A |
page.js gzip | 211 kB | 211 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 675 B | 674 B | N/A |
middleware-r..fest.js gzip | 155 B | 156 B | N/A |
middleware.js gzip | 31.4 kB | 31.4 kB | N/A |
edge-runtime..pack.js gzip | 844 B | 844 B | ✓ |
Overall change | 844 B | 844 B | ✓ |
Next Runtimes
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 395 kB | 395 kB | ✓ |
app-page-exp..prod.js gzip | 133 kB | 133 kB | ✓ |
app-page-tur..prod.js gzip | 145 kB | 145 kB | ✓ |
app-page-tur..prod.js gzip | 141 kB | 141 kB | ✓ |
app-page.run...dev.js gzip | 384 kB | 384 kB | ✓ |
app-page.run..prod.js gzip | 129 kB | 129 kB | ✓ |
app-route-ex...dev.js gzip | 39.4 kB | 39.4 kB | ✓ |
app-route-ex..prod.js gzip | 25.7 kB | 25.7 kB | ✓ |
app-route-tu..prod.js gzip | 25.7 kB | 25.7 kB | ✓ |
app-route-tu..prod.js gzip | 25.5 kB | 25.5 kB | ✓ |
app-route.ru...dev.js gzip | 39.1 kB | 39.1 kB | ✓ |
app-route.ru..prod.js gzip | 25.5 kB | 25.5 kB | ✓ |
dist_client_...dev.js gzip | 356 B | 356 B | ✓ |
dist_client_...dev.js gzip | 349 B | 349 B | ✓ |
pages-api-tu..prod.js gzip | 9.72 kB | 9.72 kB | ✓ |
pages-api.ru...dev.js gzip | 11.8 kB | 11.8 kB | ✓ |
pages-api.ru..prod.js gzip | 9.72 kB | 9.72 kB | ✓ |
pages-turbo...prod.js gzip | 22 kB | 22 kB | ✓ |
pages.runtim...dev.js gzip | 31.6 kB | 31.6 kB | ✓ |
pages.runtim..prod.js gzip | 22 kB | 22 kB | ✓ |
server.runti..prod.js gzip | 61.2 kB | 61.2 kB | ✓ |
Overall change | 1.68 MB | 1.68 MB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js sebbie/valid-url-turbopack | Change | |
---|---|---|---|
0.pack gzip | 2.12 MB | 2.12 MB | |
index.pack gzip | 76 kB | 76.1 kB | |
Overall change | 2.19 MB | 2.19 MB |
Diff details
Diff for main-HASH.js
Diff too large to display
66896c8
to
ad15da7
Compare
@@ -84,7 +84,7 @@ pub async fn resolve_source_map_sources( | |||
.await? | |||
{ | |||
let path_str = path.to_string().await?; | |||
let source = format!("{SOURCE_MAP_PREFIX}{}", path_str); | |||
let source = format!("{SOURCE_URL_PROTOCOL}{}", path_str); |
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.
The three slashes are missing here, intentional?
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.
No. Do you know how to hit this branch? Seems like we don't have any tests for it.
crates/napi/src/next_api/project.rs
Outdated
@@ -1266,8 +1266,8 @@ pub async fn project_trace_source( | |||
Some(source_file.to_string()), | |||
false, | |||
) | |||
} else if let Some(source_file) = original_file.strip_prefix(SOURCE_MAP_PREFIX) { | |||
// All other code like turbopack://[turbopack] is internal code | |||
} else if let Some(source_file) = original_file.strip_prefix(SOURCE_URL_PROTOCOL) { |
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.
The three slashes are missing here, intentional?
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.
No but I also don't understand this branch. Isn't this saying that we resolve a turbopack:///[turbopack]/file.ts
to ///[turbopack]/file.ts
(or previously [turbopack]/file.ts
. That seems like it's not a desired location since it's no longer clear what [turbopack]
stands for.
turbopack://[project]/
->turbopack:///[project]/
turbopack://[turbopack]
->turbopack:///[turbopack]/
turbopack://[next]
->turbopack:///[next]/
(don't actually know where this is coming from).Entries in
sources
of a sourcemaps need to be valid relative or absolute URLs. The part after the protocol is considered the "host". If a host is encapsulated by square brackets, it is treated as an IPv6. That means that URLs liketurbopack://[project]
are not valid sinceproject
is not a valid IPv6.Source map consumers are free to ignore invalid source URLs. However, it's also valid by spec for them to throw. Both Node.js and
source-map
choose to throw while Chrome simply ignores the URL.So we need to make an effort to create valid URLs. Sourcemaps from 3rd party libraries can still be invalid which we handle during sourcemapping since #75713.
Prior art #75719 where I tried different hostnames instead. Turbopack team prefers to move
[...]
to the pathname instead.Closes https://linear.app/vercel/issue/NDX-795/