achan3/P2X-1041 new disableSharedBundles config option #9209
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
…es are manually set
…redBundles are manually set" This reverts commit fd2cdaa.
…SharedBundles config options
adelchan07
changed the title
adelchan07
changed the title
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold Bundles
Cached BundlesNo bundle changes detected. |
thebriando
reviewed
Aug 24, 2023
gorakong
reviewed
Aug 24, 2023
| await bundle( | ||
| path.join( | ||
| __dirname, | ||
| 'integration/disable-shared-bundles-false-with-min-bundle-size/index.js', |
There was a problem hiding this comment.
since disable-shared-bundles-false-with-min-bundle-size has essentially the same contents as disable-shared-bundles-false we could probably reuse one of them, and get rid of the other
gorakong
reviewed
Aug 24, 2023
|
so i think we could probably condense these tests to 4 cases: disablesharedbundles explicitly set to false:
disablesharedbundles explicitly set to true w minBundles set:
disablesharedbundles explicitly set to true w maxParallelRequests set:
disablesharedbundles explicitly set to true w both minBundles and maxParallelRequests set:
|
AGawrys
reviewed
Aug 24, 2023
...ration-tests/test/integration/disable-shared-bundles-false-with-min-bundle-size/package.json
Outdated
Show resolved
Hide resolved
|
There's a test case called |
added 2 commits
August 25, 2023 11:41
adelchan07
changed the title
scenario handled in 18f6510 |
adelchan07
requested review from
gorakong,
thebriando,
AGawrys and
devongovett
AGawrys
requested changes
Sep 8, 2023
There was a problem hiding this comment.
I would also bar // Step Remove Shared Bundles from occurring as well if disableSharedBundles is true. Because that section looks up bundles and sorts them before we even know if the bundle group contains shared bundles, which is just extra work that shouldn't occur ideally
added 4 commits
September 8, 2023 13:13
…hecks, update test accordingly
AGawrys
approved these changes
Sep 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
↪️ Pull Request
Support a new disableSharedBundles config option during bundler creation
💻 Examples
🚨 Test instructions
✔️ PR Todo