Skip to content

[iOS] Fixes FORCE_BUNDLING error when bundle js#45871

Closed
zhongwuzw wants to merge 8 commits into
facebook:mainfrom
zhongwuzw:features/fix_bundle_js
Closed

[iOS] Fixes FORCE_BUNDLING error when bundle js#45871
zhongwuzw wants to merge 8 commits into
facebook:mainfrom
zhongwuzw:features/fix_bundle_js

Conversation

@zhongwuzw
Copy link
Copy Markdown
Contributor

@zhongwuzw zhongwuzw commented Aug 2, 2024

Summary:

After enable FORCE_BUNDLING to true, build error. cc @blakef
image

image

Changelog:

[IOS] [FIXED] - Fixes FORCE_BUNDLING error when bundle js

Test Plan:

enable FORCE_BUNDLING to true, build success.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Aug 2, 2024
@blakef blakef self-assigned this Aug 2, 2024
@blakef
Copy link
Copy Markdown
Contributor

blakef commented Aug 2, 2024

Background

This was attempted a little earlier, but without success. We have to revert. The root cause is we have multiple version of commander which is causing this problem. So it'll work from some contexts and (as in this case) it'll fail:

➜  react-native git:(main) ✗ yarn why commander
yarn why v1.18.0
[1/4] 🤔  Why do we have the module "commander"...?
...
=> Found "commander@2.20.3"
info Has been hoisted to "commander"
info Reasons this module exists
   - "workspace-aggregator-1f1a3341-7aac-4ddd-9191-0221e05f6786" depends on it
   - Hoisted from "_project_#@definitelytyped#dtslint#tslint#commander"
   - Hoisted from "_project_#@react-native#bots#danger#commander"
   - Hoisted from "_project_#@react-native#community-cli-plugin#metro#metro-minify-terser#terser#commander"
...
=> Found "helloworld#commander@12.0.0"
info This module exists because "_project_#helloworld" depends on it.
...
=> Found "react-native-info#commander@12.0.0"
info This module exists because "_project_#react-native-info" depends on it.
...
=> Found "@babel/cli#commander@4.1.1"
info This module exists because "_project_#@react-native#hermes-inspector-msggen#@babel#cli" depends on it.
...
=> Found "@react-native-community/cli#commander@9.4.1"
info This module exists because "_project_#react-native#@react-native-community#cli" depends on it.
...
✨  Done in 0.47s.

If we drop the version of commander (consistently to 9.4.1), then your change will make sense without breaking parts of CI. Those other tools might need similar tweaks.

Thoughts:

Can you rebase once main is stable, and see if this passes CI? We don't currently test for this build variant on CI. Last time we tried to land this we had CircleCI and Github Actions running.

@zhongwuzw
Copy link
Copy Markdown
Contributor Author

@blakef CI is green. :)

@zhongwuzw
Copy link
Copy Markdown
Contributor Author

Seems main branch's ci is already fixed, I'll revert the Android ci changes and rebase the main.

@zhongwuzw
Copy link
Copy Markdown
Contributor Author

@blakef I updated the solution. Please review it again.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@blakef has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 8, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@blakef merged this pull request in 089e828.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @zhongwuzw in 089e828

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: Author Feedback Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants