Skip to content

feat(rust/advance-chunks): should include matched module's dependencies too#2161

Merged
hyf0 merged 1 commit intomainfrom
09-04-feat_rust_advance-chunks_should_include_matched_module_s_dependencies_too
Sep 5, 2024
Merged

feat(rust/advance-chunks): should include matched module's dependencies too#2161
hyf0 merged 1 commit intomainfrom
09-04-feat_rust_advance-chunks_should_include_matched_module_s_dependencies_too

Conversation

@hyf0
Copy link
Member

@hyf0 hyf0 commented Sep 4, 2024 •
edited
Loading

Description

Consider case

// index.js

import { a } from './a'
import './b'

console.log(a)

// a.js

export const a = 'a

// b.js

import { a } from './a'
console.log(a)

Notice that b.js rely on a.js's export. If we move b.js to a new chunk vendor.js and don't touch a.js.

The output would be

main.js

// a.js

const a = 'a'

// index.js

console.log(a)

vendor.js

import { a } from './main'
// Wrong here. Entry chunk `main.js` doesn't export `a`. Dependencies of the module in common chunk must be in common chunk too.

// b.js

console.log(a)

@hyf0 Graphite App
Copy link
Member Author

hyf0 commented Sep 4, 2024 •
edited
Loading

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

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

@hyf0 hyf0 marked this pull request as ready for review September 4, 2024 14:56
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2024 •
edited
Loading

Benchmarks Rust

  • target: main(f003965)
  • pr: 09-04-feat_rust_advance-chunks_should_include_matched_module_s_dependencies_too(8402e8e)
group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.00     53.9±0.98ms        ? ?/sec    1.02     54.7±3.22ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     77.8±1.10ms        ? ?/sec    1.00     77.6±2.32ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.03     95.1±1.12ms        ? ?/sec    1.00     92.7±1.02ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     62.6±1.12ms        ? ?/sec    1.01     63.2±1.08ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00    102.1±0.70ms        ? ?/sec    1.01    102.7±0.87ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.00    186.4±6.06ms        ? ?/sec    1.01    188.6±6.91ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.02    242.9±8.36ms        ? ?/sec    1.00    238.9±5.54ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.01    119.3±1.78ms        ? ?/sec    1.00    118.2±1.15ms        ? ?/sec
bundle/bundle@threejs                                               1.02     33.7±0.97ms        ? ?/sec    1.00     33.0±0.30ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.01     80.5±2.43ms        ? ?/sec    1.00     79.9±1.70ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.00     97.9±1.22ms        ? ?/sec    1.02     99.5±2.96ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.00     42.4±0.70ms        ? ?/sec    1.00     42.3±0.25ms        ? ?/sec
bundle/bundle@threejs10x                                            1.00    360.9±5.67ms        ? ?/sec    1.02    368.0±4.18ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.00    965.4±9.27ms        ? ?/sec    1.02   980.7±15.38ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00  1221.4±14.38ms        ? ?/sec    1.01  1231.9±10.55ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.00    427.8±2.48ms        ? ?/sec    1.01    433.3±8.66ms        ? ?/sec
remapping/remapping                                                 1.02     32.7±0.15ms        ? ?/sec    1.00     32.1±0.17ms        ? ?/sec
remapping/render-chunk-remapping                                    1.00     82.3±0.76ms        ? ?/sec    1.01     83.5±0.34ms        ? ?/sec
scan/scan@rome-ts                                                   1.01     84.0±1.15ms        ? ?/sec    1.00     83.5±1.08ms        ? ?/sec
scan/scan@threejs                                                   1.00     25.6±0.35ms        ? ?/sec    1.01     25.9±0.83ms        ? ?/sec
scan/scan@threejs10x                                                1.00    262.0±1.61ms        ? ?/sec    1.00    260.9±2.31ms        ? ?/sec

@hyf0 hyf0 mentioned this pull request Sep 5, 2024
Merged
@hyf0 hyf0 self-assigned this Sep 5, 2024
Base automatically changed from 09-04-refactor_rust_code-splitting_pre_compute_module_s_dependencies to main September 5, 2024 07:08
@hyf0 hyf0 force-pushed the 09-04-feat_rust_advance-chunks_should_include_matched_module_s_dependencies_too branch from 48635c9 to 8402e8e Compare September 5, 2024 07:14
@netlify
Copy link

netlify bot commented Sep 5, 2024

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 8402e8e
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66d95a49d0413100087a440a

@hyf0 Graphite App
Copy link
Member Author

hyf0 commented Sep 5, 2024

Merge activity

  • Sep 5, 3:14 AM EDT: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.

@hyf0 hyf0 added this pull request to the merge queue Sep 5, 2024
Merged via the queue into main with commit a557b51 Sep 5, 2024
@hyf0 hyf0 deleted the 09-04-feat_rust_advance-chunks_should_include_matched_module_s_dependencies_too branch September 5, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees

@hyf0 hyf0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@hyf0