Skip to content
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

Delete dead concatenate_output_assets function #65337

Merged
merged 1 commit into from
May 3, 2024

Conversation

bgw
Copy link
Member

@bgw bgw commented May 3, 2024

I noticed this was dead while working on other changes to this file.

Checked that it built with:

cargo check

I noticed this was dead while working on other changes to this file.
@ijjk ijjk added created-by: Turbopack team PRs by the turbopack team Turbopack Related to Turbopack with Next.js. type: next labels May 3, 2024
Copy link
Member Author

bgw commented May 3, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bgw and the rest of your teammates on Graphite Graphite

@bgw bgw marked this pull request as ready for review May 3, 2024 20:46
@bgw bgw requested review from ijjk, shuding, huozhi and a team as code owners May 3, 2024 20:46
@ijjk
Copy link
Member

ijjk commented May 3, 2024

Stats from current PR

Default Build
General
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
buildDuration 16.6s 13.8s N/A
buildDurationCached 8.1s 7.2s N/A
nodeModulesSize 360 MB 360 MB
nextStartRea..uration (ms) 410ms 413ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
1103-HASH.js gzip 31.9 kB 31.9 kB N/A
1a9f679d-HASH.js gzip 53.5 kB 53.5 kB N/A
335-HASH.js gzip 5.05 kB 5.05 kB
7953.HASH.js gzip 169 B 169 B
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 231 B 229 B N/A
main-HASH.js gzip 31.6 kB 31.6 kB N/A
webpack-HASH.js gzip 1.65 kB 1.65 kB N/A
Overall change 50.4 kB 50.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
_app-HASH.js gzip 192 B 193 B N/A
_error-HASH.js gzip 192 B 193 B N/A
amp-HASH.js gzip 507 B 511 B N/A
css-HASH.js gzip 341 B 343 B N/A
dynamic-HASH.js gzip 2.52 kB 2.52 kB
edge-ssr-HASH.js gzip 266 B 265 B N/A
head-HASH.js gzip 362 B 365 B N/A
hooks-HASH.js gzip 392 B 392 B
image-HASH.js gzip 4.27 kB 4.27 kB
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.69 kB 2.7 kB N/A
routerDirect..HASH.js gzip 329 B 328 B N/A
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 324 B 324 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 8.27 kB 8.27 kB
Client Build Manifests
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
_buildManifest.js gzip 483 B 484 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
index.html gzip 528 B 529 B N/A
link.html gzip 541 B 540 B N/A
withRouter.html gzip 524 B 523 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
edge-ssr.js gzip 94.8 kB 94.8 kB N/A
page.js gzip 182 kB 182 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
middleware-b..fest.js gzip 622 B 624 B N/A
middleware-r..fest.js gzip 156 B 156 B
middleware.js gzip 25.7 kB 25.7 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 995 B 995 B
Next Runtimes
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
app-page-exp...dev.js gzip 171 kB 171 kB
app-page-exp..prod.js gzip 98.8 kB 98.8 kB
app-page-tur..prod.js gzip 100 kB 100 kB
app-page-tur..prod.js gzip 94.6 kB 94.6 kB
app-page.run...dev.js gzip 157 kB 157 kB
app-page.run..prod.js gzip 93.3 kB 93.3 kB
app-route-ex...dev.js gzip 21.5 kB 21.5 kB
app-route-ex..prod.js gzip 15.3 kB 15.3 kB
app-route-tu..prod.js gzip 15.3 kB 15.3 kB
app-route-tu..prod.js gzip 15 kB 15 kB
app-route.ru...dev.js gzip 21.3 kB 21.3 kB
app-route.ru..prod.js gzip 15 kB 15 kB
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 21.5 kB 21.5 kB
pages.runtim...dev.js gzip 22.1 kB 22.1 kB
pages.runtim..prod.js gzip 21.5 kB 21.5 kB
server.runti..prod.js gzip 51.7 kB 51.7 kB
Overall change 964 kB 964 kB
build cache
vercel/next.js canary vercel/next.js bgw/rm-dead-concatenate-output-assets Change
0.pack gzip 1.62 MB 1.62 MB N/A
index.pack gzip 113 kB 113 kB N/A
Overall change 0 B 0 B
Diff details
Diff for edge-ssr.js

Diff too large to display

Commit: a937fff

@bgw bgw merged commit 8dc0eff into canary May 3, 2024
80 checks passed
@bgw bgw deleted the bgw/rm-dead-concatenate-output-assets branch May 3, 2024 21:20
Copy link
Member Author

bgw commented May 3, 2024

Merge activity

  • May 3, 5:20 PM EDT: @bgw merged this pull request with Graphite.

bgw added a commit to vercel/turbo that referenced this pull request May 8, 2024
…allsites (#8087)

### Description

Most `#[turbo_tasks::value(transparent)]` types leave their inner value as private. I think this okay, but because rustdoc hides private fields by default, it makes it hard to understand at a glance the contained value, without scrolling down to the `impl` of `VcValueType`. This PR generates documentation for these.

![Screenshot 2024-05-02 at 10 27 47 PM](https://github.com/vercel/turbo/assets/180404/c3929d1f-7d96-4908-a91a-83b32c8b380a)

<details>
<summary>Also checked that it works if there's an existing doc comment, extending rather than replacing the existing documentation.</summary>

![Screenshot 2024-05-02 at 10 25 06 PM](https://github.com/vercel/turbo/assets/180404/0614d96b-f715-4eb2-b555-0fea18d14474)
</details>

**Alternative:** We could fail if the field is non-`pub`, and update the callsites. Let me know if this is preferred. The contained fields are *basically* public anyways, as they can be accessed via `Vc`'s APIs.

While modifying this code, I realized that we don't generate an error if `#[turbo_tasks::value(transparent)]` would be a no-op. The second commit in this PR adds an error and updates the callsites. This also exposed some issues in the next.js repository, which are fixed in vercel/next.js#65337 .

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->


Closes PACK-3038
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
created-by: Turbopack team PRs by the turbopack team locked Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants