Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/__tests__/browser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,19 @@ describe('Browser', () => {
expect(uuid2).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i);
expect(uuid1).not.toEqual(uuid2);
});

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\").");
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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').

Suggested change
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").'
);

Copilot uses AI. Check for mistakes.
} finally {
global.crypto = tmp;
}
});
Comment on lines 165 to 174
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

@coderabbitai is it even easily possible to write a React Native test in a Node.js env?

Copy link

Choose a reason for hiding this comment

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

🧩 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:

  1. The new implementation is environment-agnostic: After this PR, the uuid module simply checks for crypto.randomUUID at runtime—there's no React Native-specific code path anymore.

  2. The browser tests already cover both scenarios:

    • Success case: UUID generation when crypto.randomUUID is available (Line 158-163)
    • Error case: Throws error when crypto.randomUUID is unavailable (the new test in this PR, Line 165-179)
  3. 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');

Copy link
Member

Choose a reason for hiding this comment

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

So we won't add a test here, the error message plus breaking change entry should be enough.

Copy link

Choose a reason for hiding this comment

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

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!

});
32 changes: 9 additions & 23 deletions src/uuid.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
let uuid: () => string;

if (process.env.PARSE_BUILD === 'weapp') {
uuid = function () {
const s: string[] = [];
const hexDigits = '0123456789abcdef';

for (let i = 0; i < 36; i++) {
s[i] = hexDigits.substr(Math.floor(Math.random() * 0x10), 1);
}

s[14] = '4'; // bits 12-15 of the time_hi_and_version field to 0010
s[19] = hexDigits.substr((Number(s[19]) & 0x3) | 0x8, 1); // bits 6-7 of the clock_seq_hi_and_reserved to 01
s[8] = s[13] = s[18] = s[23] = '-';
return s.join('');
};
} else if (process.env.PARSE_BUILD === 'browser') {
// Use native crypto.randomUUID() from the Web Crypto API in browsers
uuid = () => globalThis.crypto.randomUUID();
} else {
// Use Node.js crypto.randomUUID() for Node and React Native builds
uuid = require('crypto').randomUUID;
}
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();
};
Comment on lines 1 to 9
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no way to test in those environments, the test covers them as best as it can.

Copy link
Member

Choose a reason for hiding this comment

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

@coderabbitai how could this be tested? Or is it save to assume that no test is needed?

Copy link

Choose a reason for hiding this comment

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

🧩 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

  1. Error throwing (✓ already covered): The new test in src/__tests__/browser-test.js:167-173 validates that the error is thrown when crypto is unavailable.

  2. Polyfill scenario: You could add a test that mocks global.crypto.randomUUID to 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 provide crypto.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

  1. Documentation: The PR description should explicitly state this is a breaking change for WeChat Mini Programs that don't polyfill crypto.randomUUID.

  2. 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()
      };
    }
  3. 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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link

Choose a reason for hiding this comment

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

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!

Copy link
Member

Choose a reason for hiding this comment

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

@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?

Copy link

Choose a reason for hiding this comment

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

@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.


export default uuid;
2 changes: 1 addition & 1 deletion types/uuid.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
declare let uuid: () => string;
declare const uuid: () => string;
export default uuid;
Loading