Skip to content

chore: Use metro extraNodeModules instead of rn-nodeify #15378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Cal-L
Copy link
Contributor

@Cal-L Cal-L commented May 15, 2025

Description

This PR removes rn-nodeify in favor of the more modern extraNodeModules field in metro for React Native. This provides a bit of a cleaner interface to work with and doesn't require mutation of node_modules
Reference - https://gist.github.com/parshap/e3063d9bf6058041b34b26b7166fd6bd#metroconfigjs-and-extranodemodules

Related issues

Fixes:

Manual testing steps

App should still function as normally. Ex. To confirm crypto polyfill, console log the import of crypto and notice that it should match react-native-crypto, which we're using to fill crypto

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@Cal-L Cal-L requested a review from a team as a code owner May 15, 2025 22:52
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label May 15, 2025
@@ -31,6 +31,17 @@ module.exports = function (baseConfig) {
...defaultConfig.resolver.extraNodeModules,
crypto: require.resolve('react-native-crypto'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be replaced with react-native-quick-crypto if we can

@Cal-L Cal-L added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 15, 2025
@github-project-automation github-project-automation bot moved this to Needs dev review in PR review queue May 15, 2025
@Cal-L Cal-L added Run Smoke E2E Requires smoke E2E testing No QA Needed Apply this label when your PR does not need any QA effort. labels May 15, 2025
Copy link
Contributor

github-actions bot commented May 15, 2025

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4e8ff38
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/efcacad6-afc5-4aa1-8db6-0caa30b65993

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) No QA Needed Apply this label when your PR does not need any QA effort. Run Smoke E2E Requires smoke E2E testing team-mobile-platform Mobile Platform team
Projects
Status: Needs dev review
Development

Successfully merging this pull request may close these issues.

2 participants