fix(core): add URL polyfill for signer in react native #11362
Conversation
This reverts commit 525befc.
| @@ -67,6 +67,7 @@ | |||
| "@aws-sdk/credential-provider-cognito-identity": "3.6.1", | |||
| "@aws-sdk/types": "3.6.1", | |||
| "@aws-sdk/util-hex-encoding": "3.6.1", | |||
| "react-native-url-polyfill": "^1.3.0", | |||
There was a problem hiding this comment.
How large is this? does it have react-native as peerDependency?
There was a problem hiding this comment.
This dependency does not affect the web bundle size. It does not introcuce much extra install size either because the core package already has react-native dependency:
amplify-js/packages/core/package.json
Line 55 in 6c1309a
But front-end user might install react-native dependency. But it's not additional either, the AWS SDK client from the core brings in the react-native already:
└─┬ @aws-amplify/core@5.2.0
└─┬ @aws-sdk/client-cloudwatch-logs@3.6.1
└─┬ @aws-sdk/middleware-retry@3.6.1
└─┬ react-native-get-random-values@1.8.0
└── react-native@0.71.6
There was a problem hiding this comment.
The react-native dependency is also deduped, so no extra install side either.
amplify-storage-ts@0.1.0 /Users/zheallan/workspace/tmp/amplify-storage-ts
├─┬ @aws-amplify/core@5.2.0
│ └─┬ @aws-sdk/client-cloudwatch-logs@3.6.1
│ └─┬ @aws-sdk/middleware-retry@3.6.1
│ └─┬ react-native-get-random-values@1.8.0
│ └── react-native@0.71.6 deduped
└─┬ aws-amplify@5.2.1-fix-rn-signer-url.dbacebc.7
└─┬ @aws-amplify/core@5.2.1-fix-rn-signer-url.dbacebc.7
└─┬ react-native-url-polyfill@1.3.0
└── react-native@0.71.6
| @@ -10,6 +10,11 @@ jest.mock('react-native', () => ({ | |||
| removeEventListener: jest.fn(), | |||
| }, | |||
| })); | |||
| jest.mock('react-native-url-polyfill', () => { | |||
| @@ -16,4 +16,9 @@ module.exports = { | |||
| functions: 95, | |||
| }, | |||
| }, | |||
| // Fix react native url polyfill transform issue of import statement. | |||
| // See: https://thymikee.github.io/jest-preset-angular/docs/11.0/guides/troubleshooting/#unexpected-token-importexportother | |||
| transformIgnorePatterns: [ | |||
There was a problem hiding this comment.
Oh hum this was still needed despite the mocking? Ah well good work either way figuring this out.
* fix(core): add RN url polyfill for signer code path * test(notification): unblock unit test failure of rn url polyfill
Description of changes
Manual integration test on the signer refactor indicates the URL polyfill is required for React Native. This change adds the ReactNative URL polyfill. Additional change to
notificationsunit test is required to fix a tranform issue.Issue #, if available
React Native sample throws error in current main:
Description of how you validated changes
This change has been released under the NPM tagged release fix-rn-signer-url, and manually validated in all providers using signer class in RN CLI and Expo and web/SSR.
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.