-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
refactor(turbo-tasks): Derive NonLocalValue by default in value/value_trait macros #73766
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ijjk
added
created-by: Turbopack team
PRs by the Turbopack team.
Turbopack
Related to Turbopack with Next.js.
labels
Dec 10, 2024
bgw
changed the title
codemod(turbo-tasks): Derive NonLocalValue by default in value/value_trait macros
refactor(turbo-tasks): Derive NonLocalValue by default in value/value_trait macros
Dec 10, 2024
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
buildDuration | 18.4s | 15.6s | N/A |
buildDurationCached | 14.7s | 12.3s | N/A |
nodeModulesSize | 410 MB | 410 MB | N/A |
nextStartRea..uration (ms) | 466ms | 470ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
1187-HASH.js gzip | 50.8 kB | 50.8 kB | N/A |
8276.HASH.js gzip | 169 B | 168 B | N/A |
8377-HASH.js gzip | 5.36 kB | 5.36 kB | N/A |
bccd1874-HASH.js gzip | 53 kB | 53 kB | N/A |
framework-HASH.js gzip | 57.5 kB | 57.5 kB | N/A |
main-app-HASH.js gzip | 232 B | 235 B | N/A |
main-HASH.js gzip | 34 kB | 34 kB | N/A |
webpack-HASH.js gzip | 1.71 kB | 1.71 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js bgw/non-local-default | 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 bgw/non-local-default | Change | |
---|---|---|---|
_app-HASH.js gzip | 193 B | 193 B | ✓ |
_error-HASH.js gzip | 193 B | 193 B | ✓ |
amp-HASH.js gzip | 512 B | 510 B | N/A |
css-HASH.js gzip | 343 B | 342 B | N/A |
dynamic-HASH.js gzip | 1.84 kB | 1.84 kB | ✓ |
edge-ssr-HASH.js gzip | 265 B | 265 B | ✓ |
head-HASH.js gzip | 363 B | 362 B | N/A |
hooks-HASH.js gzip | 393 B | 392 B | N/A |
image-HASH.js gzip | 4.49 kB | 4.49 kB | N/A |
index-HASH.js gzip | 268 B | 268 B | ✓ |
link-HASH.js gzip | 2.35 kB | 2.34 kB | N/A |
routerDirect..HASH.js gzip | 328 B | 328 B | ✓ |
script-HASH.js gzip | 397 B | 397 B | ✓ |
withRouter-HASH.js gzip | 323 B | 326 B | N/A |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 3.59 kB | 3.59 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
_buildManifest.js gzip | 747 B | 746 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
index.html gzip | 524 B | 522 B | N/A |
link.html gzip | 539 B | 537 B | N/A |
withRouter.html gzip | 520 B | 519 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
edge-ssr.js gzip | 128 kB | 128 kB | N/A |
page.js gzip | 203 kB | 203 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 669 B | 668 B | N/A |
middleware-r..fest.js gzip | 155 B | 156 B | N/A |
middleware.js gzip | 31.2 kB | 31.2 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 bgw/non-local-default | Change | |
---|---|---|---|
523-experime...dev.js gzip | 322 B | 322 B | ✓ |
523.runtime.dev.js gzip | 314 B | 314 B | ✓ |
app-page-exp...dev.js gzip | 323 kB | 323 kB | ✓ |
app-page-exp..prod.js gzip | 127 kB | 127 kB | ✓ |
app-page-tur..prod.js gzip | 140 kB | 140 kB | ✓ |
app-page-tur..prod.js gzip | 135 kB | 135 kB | ✓ |
app-page.run...dev.js gzip | 313 kB | 313 kB | ✓ |
app-page.run..prod.js gzip | 123 kB | 123 kB | ✓ |
app-route-ex...dev.js gzip | 37.3 kB | 37.3 kB | ✓ |
app-route-ex..prod.js gzip | 25.4 kB | 25.4 kB | ✓ |
app-route-tu..prod.js gzip | 25.4 kB | 25.4 kB | ✓ |
app-route-tu..prod.js gzip | 25.2 kB | 25.2 kB | ✓ |
app-route.ru...dev.js gzip | 38.9 kB | 38.9 kB | ✓ |
app-route.ru..prod.js gzip | 25.2 kB | 25.2 kB | ✓ |
pages-api-tu..prod.js gzip | 9.67 kB | 9.67 kB | ✓ |
pages-api.ru...dev.js gzip | 11.6 kB | 11.6 kB | ✓ |
pages-api.ru..prod.js gzip | 9.66 kB | 9.66 kB | ✓ |
pages-turbo...prod.js gzip | 21.7 kB | 21.7 kB | ✓ |
pages.runtim...dev.js gzip | 27.4 kB | 27.4 kB | ✓ |
pages.runtim..prod.js gzip | 21.7 kB | 21.7 kB | ✓ |
server.runti..prod.js gzip | 916 kB | 916 kB | ✓ |
Overall change | 2.36 MB | 2.36 MB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js bgw/non-local-default | Change | |
---|---|---|---|
0.pack gzip | 2.05 MB | 2.05 MB | N/A |
index.pack gzip | 72.4 kB | 72.5 kB | |
Overall change | 72.4 kB | 72.5 kB |
Diff details
Diff for main-HASH.js
Diff too large to display
Tests Passed |
bgw
force-pushed
the
bgw/impl-more-non-local-value
branch
from
December 10, 2024 23:15
459935d
to
2414df5
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 10, 2024 23:15
abbdd33
to
bc4a70d
Compare
bgw
force-pushed
the
bgw/impl-more-non-local-value
branch
from
December 10, 2024 23:44
2414df5
to
a0576b7
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 10, 2024 23:45
bc4a70d
to
60e3fcd
Compare
mischnic
reviewed
Dec 11, 2024
bgw
force-pushed
the
bgw/impl-more-non-local-value
branch
from
December 11, 2024 18:41
a0576b7
to
d0e125c
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 11, 2024 18:41
60e3fcd
to
20df3b7
Compare
bgw
force-pushed
the
bgw/impl-more-non-local-value
branch
from
December 11, 2024 18:59
d0e125c
to
98314ca
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 11, 2024 18:59
20df3b7
to
56f8361
Compare
bgw
changed the base branch from
bgw/impl-more-non-local-value
to
graphite-base/73766
December 11, 2024 19:23
bgw
force-pushed
the
graphite-base/73766
branch
from
December 11, 2024 19:23
98314ca
to
3b136b6
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 11, 2024 19:23
56f8361
to
fa5306c
Compare
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 11, 2024 19:24
fa5306c
to
1ded04c
Compare
mischnic
approved these changes
Dec 12, 2024
``` fastmod --accept-all '#\[turbo_tasks::value(?<trait>_trait)?\(((?<prefix>.*), )?non_local(?<suffix>.*)\)\]' '#[turbo_tasks::value${trait}(${prefix}${suffix})]' fastmod --accept-all '#\[turbo_tasks::value(?<trait>_trait)?\(\)\]' '#[turbo_tasks::value${trait}]' ```
Ran this a bunch of times: ``` cargo check --message-format=json 2>/dev/null | jq -r '. | select(.message.children != null) | .message.children[] | select(.spans != null) | .spans[] | select(.expansion.macro_decl_name == "#[derive(turbo_tasks::NonLocalValue)]") | .file_name + " " + (.line_start|tostring) + " " + (.line_end|tostring)' | sort -u | xargs -I{} bash -c 'args=($1); file="${args[0]}"; line="${args[1]}"; echo "Processing: $file $line" >&2; sed -i "${line}s/#\[turbo_tasks::value(\([^)]*\))\]/#[turbo_tasks::value(\1, local)]/; ${line}s/#\[turbo_tasks::value\]/#[turbo_tasks::value(local)]/" "$file"' -- {} && \ cargo check --message-format=json 2>/dev/null | jq -r '. | select(.message.children != null) | .message.children[] | select(.spans != null) | .spans[] | select(.expansion.macro_decl_name == "#[turbo_tasks::value_trait]") | .file_name + " " + (.line_start|tostring) + " " + (.line_end|tostring)' | sort -u | xargs -I{} bash -c 'args=($1); file="${args[0]}"; line="${args[1]}"; echo "Processing: $file $line" >&2; sed -i "${line}s/#\[turbo_tasks::value_trait(\([^)]*\))\]/#[turbo_tasks::value_trait(\1, local)]/; ${line}s/#\[turbo_tasks::value_trait\]/#[turbo_tasks::value_trait(local)]/" "$file"' -- {} ```
bgw
force-pushed
the
bgw/non-local-default
branch
from
December 12, 2024 23:07
1ded04c
to
e38ba3b
Compare
Merge activity
|
This was referenced Dec 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recommendation: Review the individual commits in order!
What is NonLocalValue? https://turbopack-rust-docs.vercel.sh/rustdoc/turbo_tasks/trait.NonLocalValue.html
#[turbo_tasks::value]
and#[turbo_tasks::value_trait]
to deriveNonLocalValue
, with an opt-out (previously, this was just opt-in).NonLocalValue
(as this is now the default behavior)NonLocalValue
to a couple more common types.local
annotations to structs and traits with errors.Normally I'd split this sort of thing up across a few PRs, but steps 1, 2, and 4 must be atomic (must land together).
Removing pre-existing opt-ins was done with a couple regexes:
Adding
local
annotations to structs/enums and traits was done with this kludge (partially sourced from chatgpt):Which I just ran a bunch of times until
cargo check
no longer generated any errors.Closes PACK-3647