fix(deps): request consumers to install react-native polyfills#2191
fix(deps): request consumers to install react-native polyfills#2191
Conversation
The AWS SDK for JavaScript no longer imports react-native polyfills, and expect them to be imported by the app using the SDK in react-native environment. I'll evaluate the behavior if the react-native app doesn't contain react-native polyfills, and update this PR. |
|
Ah right I see - sorry it didn't click that this change was an add, not a modify 😅 |
|
This PR is currently pending review from amplify-js team. |
|
Notes from the meeting on 3/31 with Amplify team:
|
31a2f8b to
5baa663
Compare
Removed in
This isn't possible, as it's an implementation detail in https://github.com/uuidjs/uuid/blob/91805f665c38b691ac2cbda56a99231432b00a1a/src/rng-browser.js#L21-L25 I've updated README to share react-native polyfills import example instead |
Related to aws#1536 Co-authored-by: Nicolas Perraut <n.perraut@gmail.com>
…dom-values optional peerDep
Co-authored-by: Felipe César <felipecesr@gmail.com>
Declaring react-native polyfill in peerDependenciesMeta will just remove the warning, but will still install peerDependencies
npm v7 does not install optional peerDependencies npm/arborist#138
In the testing performed by Amplify, the optional dependencies were installed by default using npm 7.7.6 We're removing optional peerDeps for react-native polyfills as of now, to be added in a separate PR.
7516038 to
d3fa8a1
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
iartemiev
left a comment
There was a problem hiding this comment.
Approved after testing the effect of this PR on Amplify RN apps.
AllanZhengYP
left a comment
There was a problem hiding this comment.
So nice to get rid of react-native from most users' dependencies.
No longer required aws#2191
|
So much appreciation for this PR! Unblocked on my work now :D |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |

Co-authored-by: Nicolas Perraut n.perraut@gmail.com
Issue
Related to #1536
Replaces #2108
Fixes: #1797
Fixes: #2051
Fixes: #2192
Fixes: #2199
Fixes: #2219
Description
This PR marks react-native-get-random-values as optional peer dependency to prevent install on non React Native environments.
Testing
Verified that ReactNative E2E tests are successful.
Command line output
Details
react-native-e2e-tests.mov
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.