@samitouri / QOS-React-2 / commits / 395e2fc6a0

[chore] Fix TypeError in shouldSkipBundle During yarn build-for-devtools-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](https://github.com/facebook/react/issues/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](https://github.com/facebook/react/issues/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.

nakjun committed Jun 21, 2024 at 05:12 UTC 395e2fc6a0f6b90b4b7b1319a4029def73118518
1 file changed +5 -3
scripts/rollup/build.js
+5 -3
@@ -85,9 +85,11 @@ function parseRequestedNames(names, toCase) {
85 return result;
86 }
87
88 +const argvType = Array.isArray(argv.type) ? argv.type : [argv.type];
89 const requestedBundleTypes = argv.type
89 - ? parseRequestedNames([argv.type], 'uppercase')
90 + ? parseRequestedNames(argvType, 'uppercase')
91 : [];
92 +
93 const requestedBundleNames = parseRequestedNames(argv._, 'lowercase');
94 const forcePrettyOutput = argv.pretty;
95 const isWatchMode = argv.watch;
@@ -528,8 +530,8 @@ function shouldSkipBundle(bundle, bundleType) {
530 return true;
531 }
532 if (requestedBundleTypes.length > 0) {
531 - const isAskingForDifferentType = requestedBundleTypes.every(
532 - requestedType => bundleType.indexOf(requestedType) === -1
533 + const isAskingForDifferentType = requestedBundleTypes.some(
534 + requestedType => !bundleType.includes(requestedType)
535 );
536 if (isAskingForDifferentType) {
537 return true;