fix: crypto.randomUUID causes React-Native build failures#2858
fix: crypto.randomUUID causes React-Native build failures#2858dplewis wants to merge 18 commits intoparse-community:alphafrom
crypto.randomUUID causes React-Native build failures#2858Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughConsolidated UUID generation into a single Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2858 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 6235 6225 -10
Branches 1493 1491 -2
=========================================
- Hits 6235 6225 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @README.md:
- Line 10: The Node.js version badge excludes v18 while the compatibility table
(lines ~54–57) still lists v18; update the badge string in README.md to include
18 (e.g., change "20,_22,_24" to "18,_20,_22,_24") and verify the compatibility
table and any other references now match the badge; alternatively, if you intend
to drop v18 support, remove it from the compatibility table instead—ensure both
the badge and table are consistent.
- Around line 97-99: Replace the ES6 import of the polyfill with the same
CommonJS style used elsewhere: change the line that imports
'react-native-random-uuid' so it uses require() rather than import, ensuring the
polyfill still provides crypto.randomUUID() globally; keep the surrounding
comment about Parse version comparison as-is until the exact version is
confirmed.
In @src/uuid.ts:
- Line 1: The uuid helper currently calls crypto.randomUUID() directly which
throws a cryptic ReferenceError when crypto or crypto.randomUUID is missing;
update the uuid function to perform a runtime check (typeof crypto !==
'undefined' && typeof crypto.randomUUID === 'function') and throw a clear,
actionable Error if unavailable that explains how to polyfill in React Native
(mention importing "react-native-random-uuid" after Parse.setAsyncStorage() and
link to the README), otherwise return crypto.randomUUID(); reference the
existing uuid function in src/uuid.ts to locate and replace the direct call.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdsrc/uuid.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (Node 20, 20.19.0)
- GitHub Check: build (Node 24, 24.1.0)
- GitHub Check: build (Node 22, 22.12.0)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @types/uuid.d.ts:
- Line 1: The current uuid implementation relies on crypto.randomUUID() which
will crash in React Native; update the module used by the declared function uuid
to provide a RN-safe implementation by either adding a React Native specific
file (e.g., create uuid.react-native.ts or lib/react-native/uuid.js) that
returns the same `${string}-${string}-...` format using a RN-friendly polyfill
(like react-native-uuid or the uuid package), or modify src/uuid.ts to detect
React Native at runtime and fall back to the polyfill when crypto.randomUUID is
unavailable; ensure InstallationController.currentInstallationId() and other
callers import the unified uuid symbol so RN builds pick up the RN variant or
the runtime fallback.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
types/AnonymousUtils.d.tstypes/Parse.d.tstypes/uuid.d.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (Node 24, 24.1.0)
- GitHub Check: build (Node 20, 20.19.0)
- GitHub Check: build (Node 22, 22.12.0)
🔇 Additional comments (2)
types/AnonymousUtils.d.ts (1)
79-79: LGTM! Type narrowing accurately represents UUID format.The template literal type
${string}-${string}-${string}-${string}-${string}correctly represents the standard UUID format. Since this is within the_getAuthProvider()method (indicated by the_prefix as a private API), the type narrowing has minimal breaking change risk.types/Parse.d.ts (1)
36-36: LGTM! Consistent type narrowing across Parse namespace.The type change for
authData.idis consistent with the same change intypes/AnonymousUtils.d.ts, maintaining type consistency across the Parse namespace declarations.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @README.md:
- Line 10: The README badge was updated to list Node 20, 22, 24 but the
compatibility table still lists Node 18; update the README to be consistent by
either removing Node 18 from the compatibility table to match the badge, or keep
Node 18 and add an explicit note that Node 18 requires a polyfill for global
crypto.randomUUID (requirement: Node >=19 provides it natively); reference the
badge token "[![Node Version]...]" and the compatibility table section and
ensure the README clearly states the native requirement (Node ≥19) or documents
how to provide the polyfill for Node 18 users.
🧹 Nitpick comments (3)
README.md (1)
89-97: Consider documenting the crypto polyfill requirement for React Native.React Native users will now need to polyfill
crypto.randomUUIDsince this PR removes the Node crypto import. Consider adding a note here to guide users, for example:**Note:** React Native environments require a crypto polyfill. Install and import `react-native-random-uuid` or similar before initializing Parse.src/__tests__/browser-test.js (1)
165-176: Improve cleanup reliability with try-finally.The test correctly validates the error path, but if
uuidv4()unexpectedly succeeds (line 171 fails),global.cryptowon't be restored. Use afinallyblock to guarantee cleanup:♻️ Suggested refactor
it('throw error if randomUUID is not available', () => { const tmp = global.crypto; delete global.crypto; try { const uuidv4 = require('../uuid').default; uuidv4(); expect(true).toBe(false); } catch (e) { expect(e.message).toBe('crypto.randomUUID is not available. For React Native, import "react-native-random-uuid"'); + } finally { + global.crypto = tmp; } - global.crypto = tmp; });src/uuid.ts (1)
3-6: Consider a more generic error message for broader environment coverage.The error message specifically guides React Native users but this check applies to any environment without
crypto.randomUUID(e.g., older Node.js versions, older browsers). Consider a more inclusive message:💬 Suggested alternative
throw new Error( - 'crypto.randomUUID is not available. ' + - 'For React Native, import "react-native-random-uuid"' + 'crypto.randomUUID is not available. ' + + 'Please provide a polyfill (e.g., "react-native-random-uuid" for React Native)' );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdsrc/__tests__/browser-test.jssrc/uuid.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/__tests__/browser-test.js (1)
integration/test/ParseUserTest.js (2)
uuidv4(5-5)require(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (Node 24, 24.1.0)
- GitHub Check: build (Node 20, 20.19.0)
- GitHub Check: build (Node 22, 22.12.0)
🔇 Additional comments (1)
src/uuid.ts (1)
1-9: No changes needed. The implementation correctly assumescrypto.randomUUIDis available globally in Node.js 20+ (introduced without experimental flag in v19.0.0). The error handling appropriately guards against environments where it's unavailable, such as React Native, making this a robust solution for both Node.js and React Native use cases.
There was a problem hiding this comment.
Pull request overview
This pull request addresses React Native build failures caused by the use of node:crypto module by migrating to the global crypto object available in modern JavaScript environments. The change allows users to polyfill crypto.randomUUID in environments where it's not natively available.
Changes:
- Simplified UUID implementation to use global
crypto.randomUUID()with runtime availability check - Removed environment-specific UUID generation logic (weapp, browser, Node.js)
- Added error handling with guidance for React Native users when crypto.randomUUID is unavailable
- Changed type definition from
lettoconstfor consistency
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| types/uuid.d.ts | Updated type declaration from let to const to match the new implementation |
| src/uuid.ts | Replaced environment-specific UUID implementations with a single implementation using global crypto object, throws error if unavailable |
| src/tests/browser-test.js | Added test case to verify error handling when crypto.randomUUID is not available |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
can someone with priv, help merge this? |
|
@lawrenehi This requires addressing the review comments. |
|
@mtrezza What review comments? It looks like this PR was approved. |
|
@dplewis the copilot review comments that you just closed; please re-open them and do not close them but treat them like human feedback and respond to them; the agent will close them if you either addressed the feedback in a commit or provided a reasonable explanation for why you think the review feedback is incorrect. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Diamond Lewis <findlewis@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Diamond Lewis <findlewis@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/__tests__/browser-test.js`:
- Around line 165-174: The test in the it('throw error if randomUUID is not
available') block expects an outdated error string; update the assertion to
match the actual message thrown by src/uuid.ts (or refactor to import a shared
constant) so the expectation matches the runtime error from
require('../uuid').default(); specifically change the
expect(e.message).toBe(...) to the current message emitted by the uuid module
(or import/export a shared ERROR_MESSAGE constant from uuid.ts and use that in
the test) so they cannot drift.
|
@mtrezza Done |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/__tests__/browser-test.js
Outdated
| uuidv4(); | ||
| expect(true).toBe(false); | ||
| } catch (e) { | ||
| expect(e.message).toBe("crypto.randomUUID is not available in this environment. Use a UUID polyfill or environment-specific implementation (for example, in React Native you can import \"react-native-random-uuid\")."); |
There was a problem hiding this comment.
This assertion pattern (expect(true).toBe(false)) is used to indicate that the code should have thrown an error and never reached this line. However, this pattern can be confusing and doesn't provide a clear failure message if the exception is not thrown. Consider using Jest's built-in expect(...).toThrow() matcher instead, which is more idiomatic and provides better failure messages. For example: expect(() => uuidv4()).toThrow('crypto.randomUUID is not available').
| uuidv4(); | |
| expect(true).toBe(false); | |
| } catch (e) { | |
| expect(e.message).toBe("crypto.randomUUID is not available in this environment. Use a UUID polyfill or environment-specific implementation (for example, in React Native you can import \"react-native-random-uuid\")."); | |
| expect(() => uuidv4()).toThrow( | |
| 'crypto.randomUUID is not available in this environment. Use a UUID polyfill or environment-specific implementation (for example, in React Native you can import "react-native-random-uuid").' | |
| ); |
| it('throw error if randomUUID is not available', () => { | ||
| jest.resetModules(); | ||
| const tmp = global.crypto; | ||
| delete global.crypto; | ||
| try { | ||
| const uuidv4 = require('../uuid').default; | ||
| uuidv4(); | ||
| expect(true).toBe(false); | ||
| } catch (e) { | ||
| expect(e.message).toBe("crypto.randomUUID is not available in this environment. Use a UUID polyfill or environment-specific implementation (for example, in React Native you can import \"react-native-random-uuid\")."); | ||
| } finally { | ||
| global.crypto = tmp; | ||
| } | ||
| }); |
There was a problem hiding this comment.
The test coverage is missing verification that the uuid module works correctly in React Native test environments. While the browser test verifies the error case when crypto is unavailable, there should also be a test in react-native-test.js (similar to the weapp test at src/__tests__/weapp-test.js:58-62) to verify that uuid generation works properly when the crypto global is available or appropriately polyfilled in React Native environments. This would ensure the module behaves correctly across all supported environments.
There was a problem hiding this comment.
@coderabbitai is it even easily possible to write a React Native test in a Node.js env?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for React Native test files
fd -e js -e ts "react-native.*test" src/__tests__/
# Also search more broadly for any react-native references in test files
rg -l "react-native" src/__tests__/
# Look at the weapp test file to see what kind of test Copilot referenced
cat src/__tests__/weapp-test.js
Length of output: 2916
🏁 Script executed:
#!/bin/bash
# Check the content of react-native-test.js to see if uuid tests are present
cat src/__tests__/react-native-test.js
Length of output: 2682
@mtrezza Yes, it's definitely feasible to write React Native tests in a Node.js/Jest environment. The codebase already does this extensively in src/__tests__/react-native-test.js, which sets process.env.PARSE_BUILD = 'react-native' and mocks React Native-specific modules (like the EventEmitter, CryptoController, StorageController, etc.).
However, in this specific case, adding a React Native-specific UUID test would provide limited additional value because:
-
The new implementation is environment-agnostic: After this PR, the uuid module simply checks for
crypto.randomUUIDat runtime—there's no React Native-specific code path anymore. -
The browser tests already cover both scenarios:
- Success case: UUID generation when
crypto.randomUUIDis available (Line 158-163) - Error case: Throws error when
crypto.randomUUIDis unavailable (the new test in this PR, Line 165-179)
- Success case: UUID generation when
-
React Native would exhibit identical behavior: Whether running in React Native or browser, the module will either use the available
crypto.randomUUID(if polyfilled) or throw the same error (if not).
If you wanted to add a React Native test for consistency with other environments (like weapp has on Line 58-62), it would just verify that uuid generation works when crypto is available—essentially duplicating what the browser test already covers. The weapp test is simpler because it only checks uniqueness, not the error case.
🧬 Code Graph Analysis Results
[src/Parse.ts] 291-297
set EventuallyQueue(queue: EventuallyQueue) {
CoreManager.setEventuallyQueue(queue);
}
get EventuallyQueue(): EventuallyQueue {
return CoreManager.getEventuallyQueue();
}
[src/Parse.ts] 135-145
export interface RESTController {
request: (method: string, path: string, data?: any, options?: RequestOptions) => Promise<any>;
ajax: (
method: string,
url: string,
data: any,
headers?: any,
options?: FullOptions
) => Promise<any>;
handleError: (err?: any) => void;
}
[integration/test/ParseEventuallyQueueTest.js] 4-4
const Parse = require('../../node');
[integration/test/ParseLegacyTest.js] 3-3
const { Parse } = require('../../node');
[integration/test/ParseReactNativeTest.js] 3-3
const Parse = require('../../react-native');
[integration/test/ParseReactNativeTest.js] 4-4
const { resolvingPromise } = require('../../lib/react-native/promiseUtils');
There was a problem hiding this comment.
So we won't add a test here, the error message plus breaking change entry should be enough.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| const uuid = (): string => { | ||
| if (typeof crypto === 'undefined' || typeof crypto.randomUUID !== 'function') { | ||
| throw new Error( | ||
| 'crypto.randomUUID is not available in this environment. ' + | ||
| 'Use a UUID polyfill or environment-specific implementation (for example, in React Native you can import "react-native-random-uuid").' | ||
| ); | ||
| } | ||
| return crypto.randomUUID(); | ||
| }; |
There was a problem hiding this comment.
The old implementation had a custom UUID generation fallback for the 'weapp' (WeChat Mini Program) environment. The new implementation assumes that crypto.randomUUID() is available in all environments or requires a polyfill. While the test at src/__tests__/weapp-test.js:58-62 suggests UUIDs should work in weapp environments, ensure that WeChat Mini Programs actually provide the Web Crypto API with randomUUID support, or document that users need to polyfill it. If WeChat Mini Programs don't have native crypto.randomUUID support, this is a breaking change that needs to be clearly documented in the PR description and changelog.
There was a problem hiding this comment.
There is no way to test in those environments, the test covers them as best as it can.
There was a problem hiding this comment.
@coderabbitai how could this be tested? Or is it save to assume that no test is needed?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all test files related to uuid
echo "=== Finding uuid test files ==="
fd -e js -e ts uuid test
echo -e "\n=== Looking for all test files in __tests__ ==="
fd -t f . src/__tests__
echo -e "\n=== Examining weapp-test.js ==="
cat src/__tests__/weapp-test.js
echo -e "\n=== Examining browser-test.js (looking for new test) ==="
cat src/__tests__/browser-test.js
echo -e "\n=== Checking for react-native test file ==="
fd react-native-test src/__tests__
Length of output: 10643
🏁 Script executed:
#!/bin/bash
echo "=== Examining react-native-test.js ==="
cat src/__tests__/react-native-test.js
echo -e "\n=== Examining mockWeChat helper ==="
cat src/__tests__/test_helpers/mockWeChat.js
echo -e "\n=== Checking if global crypto is mocked anywhere in test setup ==="
rg -n "global\.crypto" src/__tests__/
echo -e "\n=== Checking Jest setup files ==="
fd -e json -e js jest.config . | head -5
cat jest.config.json 2>/dev/null || cat jest.config.js 2>/dev/null || echo "No Jest config found"
Length of output: 4449
@mtrezza, based on my analysis of the codebase and research into WeChat Mini Programs, here's the testing situation:
Current Testing Reality
WeChat Mini Programs don't have the Web Crypto API, and instead provide wx.getRandomValues() for secure random number generation. This means the new implementation will throw an error in real WeChat Mini Programs unless users polyfill crypto.randomUUID.
The existing test at src/__tests__/weapp-test.js:58-62 doesn't actually test the WeChat environment—it runs in Node.js (Jest), where crypto.randomUUID is available. So the test passes, but it's a false positive for the real runtime.
What CAN Be Tested
-
Error throwing (✓ already covered): The new test in
src/__tests__/browser-test.js:167-173validates that the error is thrown whencryptois unavailable. -
Polyfill scenario: You could add a test that mocks
global.crypto.randomUUIDto verify the function works when properly polyfilled:
it('works with polyfilled crypto.randomUUID in weapp', () => {
jest.resetModules();
global.crypto = {
randomUUID: jest.fn(() => '123e4567-e89b-12d3-a456-426614174000')
};
const uuidv4 = require('../uuid').default;
expect(uuidv4()).toBe('123e4567-e89b-12d3-a456-426614174000');
delete global.crypto;
});What CANNOT Be Tested (in CI)
- Real WeChat Mini Program runtime: As dplewis correctly noted, there's no way to run actual WeChat Mini Program tests in CI. You'd need:
- WeChat Developer Tools simulator
- A polyfill like
wx.getRandomValues()wrapped to providecrypto.randomUUID() - Manual testing in the WeChat environment
Is Testing Sufficient?
Yes, with caveats:
- The error path is tested ✓
- The happy path (when crypto.randomUUID exists) works in Node/browser ✓
- However, this is a breaking change for WeChat Mini Programs
Recommendations
-
Documentation: The PR description should explicitly state this is a breaking change for WeChat Mini Programs that don't polyfill
crypto.randomUUID. -
Migration guide: Document how WeChat users should polyfill, e.g.:
// Polyfill for WeChat Mini Programs if (typeof crypto === 'undefined' || !crypto.randomUUID) { global.crypto = global.crypto || {}; crypto.randomUUID = () => { // Implementation using wx.getRandomValues() }; }
-
Consider adding a polyfill test like the example above to document expected behavior.
The current test coverage validates the implementation works correctly in environments where it will work, but cannot test the environments where it will fail. That's acceptable given the constraints, but the breaking change needs clear documentation.
There was a problem hiding this comment.
Added breaking change label to PR; so this requires a major version release and breaking change entry. What would be a concise entry without adding a whole code block?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@coderabbitai Added breaking change label to PR; so this requires a major version release and breaking change entry. What would be a concise entry without adding a whole code block?
There was a problem hiding this comment.
@mtrezza, here are a few concise breaking change entry options:
Option 1 (Most concise):
**Breaking:** UUID generation now requires `crypto.randomUUID()` to be available. Environments without native support (such as WeChat Mini Programs) must provide a polyfill. See error message for environment-specific polyfill recommendations.
Option 2 (With more context):
**Breaking:** Removed custom UUID fallback for WeChat Mini Programs. UUID generation now depends on `crypto.randomUUID()` being available globally. Environments without native Web Crypto API support (WeChat Mini Programs, older React Native versions) must polyfill `crypto.randomUUID()` before using the SDK.
Option 3 (Most detailed, still concise):
**Breaking:** UUID generation refactored to use standard `crypto.randomUUID()` API. The WeChat Mini Program custom UUID implementation has been removed. Users in environments without native `crypto.randomUUID()` support must provide a polyfill (e.g., `react-native-random-uuid` for React Native, or a custom implementation wrapping `wx.getRandomValues()` for WeChat).
I'd recommend Option 1 or Option 2 as they strike the right balance between being informative and concise without overwhelming readers with implementation details.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Diamond Lewis <findlewis@gmail.com>
mtrezza
added
the
state:breaking
|
@mtrezza we should add examples to this SDK, how else do we test tree shaking |
|
I think if you resolve the review comments as much as you can we can go ahead and merge. |
Pull Request
Issue
Using
node:cryptoprevented users from polyfillingrandomUUIDCloses: #2856
Approach
As of Node 18 Web Crypto API is now a global object, this global object is also included in the browser. For any other runtime it will need to be polyfilled if not supported.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.