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

Split the ES chunk module into smaller, more focused modules #3974

Merged
merged 4 commits into from
Feb 28, 2023

Conversation

alexkirsz
Copy link
Contributor

@alexkirsz alexkirsz commented Feb 27, 2023

This diff splits the turbopack-ecmascript/src/chunk/mod.rs and turbopack-ecmascript/src/loader.rs modules into smaller modules, usually centered around one type.

The goal of this PR is to make the ecmascript crate easier to navigate and review, as well as reduce the amount of merge conflicts that can arise from modifying imports in two separate diffs within the same module.

This PR also changes the visibility of most modules within the crate to be pub(crate) so they're available to the generated register function, but not accidentally leaked outside of the crate. The same is done on a per-type basis with pub(crate) or pub(super).

@vercel
Copy link

vercel bot commented Feb 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-designsystem-docs 🔄 Building (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-kitchensink-blog 🔄 Building (Inspect) Feb 28, 2023 at 9:56AM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 28, 2023 at 9:56AM (UTC)
7 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 28, 2023 at 9:56AM (UTC)

@alexkirsz
Copy link
Contributor Author

alexkirsz commented Feb 27, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2023

🟢 CI successful 🟢

Thanks

@alexkirsz alexkirsz force-pushed the alexkirsz/web-616-split-ecmascript-modules branch 2 times, most recently from 9ec7680 to a2a5e3e Compare February 27, 2023 15:06
@alexkirsz alexkirsz changed the base branch from alexkirsz/web-616-remove-cell_local-in-favor-of-more to main February 27, 2023 15:24
@alexkirsz alexkirsz force-pushed the alexkirsz/web-616-split-ecmascript-modules branch from a2a5e3e to 10de7af Compare February 27, 2023 15:24
Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this only moves stuff around, I'm happy when the compiler is happy.

CI doesn't see to be happy right now

@alexkirsz alexkirsz force-pushed the alexkirsz/web-616-split-ecmascript-modules branch from 8f76720 to f1163f0 Compare February 28, 2023 09:55
@alexkirsz alexkirsz added the pr: automerge Kodiak will merge these automatically after checks pass label Feb 28, 2023
@kodiakhq kodiakhq bot merged commit 489c730 into main Feb 28, 2023
@kodiakhq kodiakhq bot deleted the alexkirsz/web-616-split-ecmascript-modules branch February 28, 2023 10:17
@github-actions
Copy link
Contributor

Benchmark for db50397

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 12.69ms ± 0.13ms 12.60ms ± 0.08ms -0.71%
bench_hmr_to_commit/Turbopack RCC/1000 modules 15.59ms ± 0.11ms 15.42ms ± 0.17ms -1.07%
bench_hmr_to_commit/Turbopack RSC/1000 modules 532.44ms ± 1.60ms 529.82ms ± 2.10ms -0.49%
bench_hmr_to_commit/Turbopack SSR/1000 modules 12.51ms ± 0.12ms 12.50ms ± 0.09ms -0.04%
bench_hmr_to_eval/Turbopack CSR/1000 modules 11.59ms ± 0.10ms 11.57ms ± 0.13ms -0.17%
bench_hmr_to_eval/Turbopack RCC/1000 modules 14.19ms ± 0.23ms 14.11ms ± 0.15ms -0.55%
bench_hmr_to_eval/Turbopack SSR/1000 modules 11.53ms ± 0.11ms 11.58ms ± 0.10ms +0.38%
bench_hydration/Turbopack RCC/1000 modules 3975.52ms ± 9.94ms 3975.95ms ± 7.58ms +0.01%
bench_hydration/Turbopack RSC/1000 modules 3618.58ms ± 15.19ms 3614.24ms ± 13.03ms -0.12%
bench_hydration/Turbopack SSR/1000 modules 3466.66ms ± 11.05ms 3474.07ms ± 8.81ms +0.21%
bench_startup/Turbopack CSR/1000 modules 2673.57ms ± 9.41ms 2659.63ms ± 10.52ms -0.52%
bench_startup/Turbopack RCC/1000 modules 2420.03ms ± 7.94ms 2439.58ms ± 4.71ms +0.81%
bench_startup/Turbopack RSC/1000 modules 2381.62ms ± 11.42ms 2374.93ms ± 5.30ms -0.28%
bench_startup/Turbopack SSR/1000 modules 2124.25ms ± 3.26ms 2125.59ms ± 2.35ms +0.06%

ijjk added a commit to vercel/next.js that referenced this pull request Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants