Repository navigation
Conversation
Bundles a small test app with the Metro setup of React Native 0.73 (Metro 0.80, which drops missing optional dependencies from a module's dependency map) and React Native 0.87 (fixed Metro), and runs each bundle in a fresh Node vm context. Cases cover fast-text-encoding, react-native-get-random-values and AsyncStorage being installed, missing, or replaced by the app's own polyfill or store, on runtimes with and without a native TextEncoder. Runs with `npm run test-rn-metro` and as a CI job on every PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![arnica-github-connector[bot]](https://avatars.githubusercontent.com/in/144692?s=60&v=4){kind=small}
|
coverage: 77.624%. remained the same — raju/rn-test into master |
There was a problem hiding this comment.
🟡 Changes recommended
The CI runtime is unsupported by the modern fixture, and assertions can accept failed decisions or unexpected fatal errors.
3 open findings
What changed in this PR
Adds Metro integration tests for the SDK’s optional React Native dependencies, automating regression coverage for the module-loading issue addressed by #1179.
Changes:
- Tests 14 dependency and polyfill scenarios across two Metro fixtures.
- Runs packaged SDK bundles in isolated contexts and checks runtime behavior.
- Adds a test command, CI job, and maintenance documentation.
| File | Description |
|---|---|
| tests/react-native-metro/stubs/react-native-get-random-values.js | Supplies a test-only random-values polyfill. |
| tests/react-native-metro/stubs/netinfo.js | Replaces native connectivity APIs. |
| tests/react-native-metro/stubs/async-storage-v3.js | Simulates storage with getMany. |
| tests/react-native-metro/stubs/async-storage-v2.js | Simulates storage with multiGet. |
| tests/react-native-metro/run.mjs | Installs, bundles, executes, and checks cases. |
| tests/react-native-metro/README.md | Documents usage and fixture maintenance. |
| tests/react-native-metro/metro.config.base.js | Controls optional dependency resolution. |
| tests/react-native-metro/fixtures/rn-0.87/package.json | Defines the modern Metro fixture. |
| tests/react-native-metro/fixtures/rn-0.87/package-lock.json | Locks modern fixture dependencies. |
| tests/react-native-metro/fixtures/rn-0.87/metro.config.js | Connects modern Metro to shared configuration. |
| tests/react-native-metro/fixtures/rn-0.73/package.json | Defines the older Metro fixture. |
| tests/react-native-metro/fixtures/rn-0.73/metro.config.js | Connects older Metro to shared configuration. |
| tests/react-native-metro/cases.mjs | Defines scenarios and expectations. |
| tests/react-native-metro/build.cjs | Builds bundles with each fixture. |
| tests/react-native-metro/app/test.js | Exercises decisions, events, and storage. |
| tests/react-native-metro/app/setup.js | Installs custom polyfills and error reporting. |
| tests/react-native-metro/app/index.js | Starts testing after module initialization. |
| tests/react-native-metro/app/datafile.json | Supplies feature configuration. |
| package.json | Exposes test-rn-metro. |
| .gitignore | Excludes generated test artifacts. |
| .github/workflows/javascript.yml | Adds the Metro CI job. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (!result) { | ||
| problems.push('no result'); | ||
| } else { | ||
| if (Object.keys(result.decisions).length === 0) problems.push('no decisions'); |
| } | ||
| } | ||
| } else if (expect.fatal) { | ||
| if (!fatal.some((e) => e.message.includes(expect.fatal))) { |
- Check decisions against the expected flag variations instead of only checking that decideAll() returned something. - In fatal and caught-error cases, require every recorded error to be the expected one, so an unexpected error (such as a shifted require) can't hide. - Run the CI job on Node 22; the RN 0.87 fixture's Metro requires Node ^22.13.0 || ^24.3.0 || >= 26.0.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
tests/react-native-metro/: checks how the SDK behaves in React Native apps whenfast-text-encoding,react-native-get-random-valuesand@react-native-async-storage/async-storageare installed, missing, or replaced by the app's own polyfill or store.@react-native/metro-config,metroand Babel pinned by committed lockfiles. Thereact-nativepackage isn't needed.npm run test-rn-metro):loadConfigas the RN CLI does. React Native's startup module is skipped because it needs native code.resolveRequestdecides which optional packages exist; packages that need native code resolve to small JS stand-ins.vmcontext, which has noTextEncoderorcrypto, like older Hermes. Cases that need a nativeTextEncoderadd one.requireis part of the bundle, so modules load exactly as in an app. The test app installs a non-throwingErrorUtilshandler, as React Native's startup code does, so a failedrequireoutside module loading is reported as fatal (the red box) instead of rethrown to the caller.react_native_metro_testjob runs on every PR (Ubuntu, Node 20, about a minute).The device testing for #1179 was done by hand on simulators. This automates the part that broke, Metro's bundling and runtime
requiretogether with the SDK's optional import shims, on both a buggy and a fixed Metro.tests/react-native-metro/README.mdexplains the setup, how to add a case, and how to update a fixture.Test plan
npm run test-rn-metropasses all 28 runs: 14 cases on each of RN 0.73 and RN 0.87.TextEncodermultiGetor thegetMany-basedmultiGetThe test catches the original bug. Against an SDK build with the shims bundled into the main bundle (the layout before [FSSDK-13287] Make React Native polyfills optional and isolate optional requires #1179), 5 cases fail on RN 0.73 and all pass on RN 0.87: P2, P4, P5 and P9 got a false async-storage "Module not found", and S3's netinfo
requirebecamerequire(undefined), reported as fatal.The existing checks don't pick up the new folder: lint, vitest and the platform-isolation check are unaffected. The workflow file was validated as YAML; the new job runs for the first time on this PR.
Issues
🤖 Generated with Claude Code