Fix TypeError in shouldSkipBundle During yarn build-for-devtools-dev and yarn build-for-devtools-prod Commands #29906
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.
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 inscripts/rollup/build.js
has two issues:Incorrect array handling in
parseRequestedNames
(#29613):The function incorrectly wraps the
argv.type
value in an additional array when it's already an array. This leads to aTypeError: names[i].split is not a function
whenparseRequestedNames
attempts to split the nested array, as seen in this error message:This PR fixes this by correctly handling both string and array inputs in
argv.type
:Inaccurate filtering logic in
shouldSkipBundle
(#29614):The function uses
Array.prototype.every
withindexOf
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:
In this case, even though the bundle type is
NODE_PROD
and doesn't includeNODE_DEV
, the bundle is not skipped due to the incorrect logic.This PR fixes this by replacing
every
withsome
and usingincludes
for a more accurate check: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?
Verification of
requestedBundleTypes
usage inshouldSkipBundle
:yarn build
: Verified thatrequestedBundleTypes
remains an empty array, as expected.yarn build-for-devtools
: Confirmed thatrequestedBundleTypes
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 thatrequestedBundleTypes
is now correctly passed as['NODE', 'NODE_DEV']
.Debugging of filtering logic in
shouldSkipBundle
:I added the following logging statements to the
shouldSkipBundle
function to observe its behavior during the build process: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.