Bug: Inaccurate Bundle Type Filtering in Rollup Build #29614
Labels
Status: Unconfirmed
A potential issue that we haven't yet confirmed as a bug
Comments
hoxyq
pushed a commit
that referenced
this issue
Jun 20, 2024
…ols-dev and yarn build-for-devtools-prod Commands (#29906) ## Summary Fix bundle type filtering logic to correctly handle array input in argv.type and use some with includes for accurate filtering. This addresses a TypeError encountered during yarn build-for-devtools-prod and yarn build-for-devtools-dev commands. ## Motivation The current implementation of the `shouldSkipBundle` function in `scripts/rollup/build.js` has two issues: 1. **Incorrect array handling in `parseRequestedNames`([#29613](#29613 The function incorrectly wraps the `argv.type` value in an additional array when it's already an array. This leads to a `TypeError: names[i].split is not a function` when `parseRequestedNames` attempts to split the nested array, as seen in this error message: ``` C:\Users\Administrator\Documents\새 폴더\react\scripts\rollup\build.js:76 let splitNames = names[i].split(','); ^ TypeError: names[i].split is not a function ``` This PR fixes this by correctly handling both string and array inputs in `argv.type`: ```diff - const requestedBundleTypes = argv.type - ? parseRequestedNames([argv.type], 'uppercase') + const argvType = Array.isArray(argv.type) ? argv.type : [argv.type]; + const requestedBundleTypes = argv.type + ? parseRequestedNames(argvType, 'uppercase') ``` 2. **Inaccurate filtering logic in `shouldSkipBundle`([#29614](#29614 The function uses `Array.prototype.every` with `indexOf` to check if **all** requested bundle types are missing in the current bundle type. However, when multiple bundle types are requested (e.g., `['NODE', 'NODE_DEV']`), the function should skip a bundle only if **none** of the requested types are present. The current implementation incorrectly allows bundles that match any of the requested types. To illustrate, consider the following example output: ``` requestedBundleTypes [ 'NODE', 'NODE_DEV' ] bundleType NODE_DEV isAskingForDifferentType false requestedBundleTypes [ 'NODE', 'NODE_DEV' ] bundleType NODE_PROD isAskingForDifferentType false // Incorrect behavior ``` In this case, even though the bundle type is `NODE_PROD` and doesn't include `NODE_DEV`, the bundle is not skipped due to the incorrect logic. This PR fixes this by replacing `every` with `some` and using `includes` for a more accurate check: ```diff - const isAskingForDifferentType = requestedBundleTypes.every( - requestedType => bundleType.indexOf(requestedType) === -1 - ); + const isAskingForDifferentType = requestedBundleTypes.some( + requestedType => !bundleType.includes(requestedType) + ); ``` This ensures that the bundle is skipped only if **none** of the requested types are found in the `bundleType`. This PR addresses both of these issues to ensure correct bundle type filtering in various build scenarios. ## How did you test this change? 1. **Verification of `requestedBundleTypes` usage in `shouldSkipBundle`:** * I manually tested the following scenarios: * `yarn build`: Verified that `requestedBundleTypes` remains an empty array, as expected. * `yarn build-for-devtools`: Confirmed that `requestedBundleTypes` is correctly set to `['NODE']`, as in the original implementation. * `yarn build-for-devtools-dev`: This previously failed due to the error. After the fix, I confirmed that `requestedBundleTypes` is now correctly passed as `['NODE', 'NODE_DEV']`. 2. **Debugging of filtering logic in `shouldSkipBundle`:** * I added the following logging statements to the `shouldSkipBundle` function to observe its behavior during the build process: ```javascript console.log('requestedBundleTypes', requestedBundleTypes); console.log('bundleType', bundleType); console.log('isAskingForDifferentType', isAskingForDifferentType); ``` * By analyzing the log output, I confirmed that the filtering logic now correctly identifies when a bundle should be skipped based on the requested types. This allowed me to verify that the fix enables building specific target bundles as intended.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
React version: 18.x.x up to latest
Steps To Reproduce
Clone the React repository.
Run:
yarn installRun either:
yarn build-for-devtools-devyarn build-for-devtools-prodObserve the output of the
shouldSkipBundlefunction logs within thebuild.jsscript.Link to code example:
scripts/rollup/build.jsshouldSkipBundleThe current behavior:
The
shouldSkipBundlefunction, responsible for determining whether to include a bundle in the build output, contains incorrect logic for filtering when multiple bundle types are specified. Specifically, the following line causes the issue:This code uses
Array.prototype.everywithindexOfto check if all requested bundle types are missing in the current bundle type. However, when multiple types are specified, it should skip a bundle only if none of the requested types are present.Here's an example of the incorrect behavior observed during debugging:
In this case, even though the
bundleTypeisNODE_PRODand doesn't includeNODE_DEV,isAskingForDifferentTypeis incorrectly evaluated asfalse, leading to the bundle not being skipped.The expected behavior:
The
shouldSkipBundlefunction should correctly filter bundles based on multiple requested types. In the example above, the expected behavior is thatisAskingForDifferentTypeshould betruefor theNODE_PRODbundle type, as it does not include theNODE_DEVtype.In general, the function should return
true(skip the bundle) only if none of the requested types are included in the current bundle type.The text was updated successfully, but these errors were encountered: