fix(deps): request consumers to install react-native polyfills #2191

trivikr · 2021-03-29T16:49:04Z

Co-authored-by: Nicolas Perraut [email protected]

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

$ pwd
/Users/trivikr/workspace/aws-sdk-js-v3/tests/react-native/End2End

$ node launch-app.js --local-publish
...
 PASS  e2e/End2EndTest.spec.js (14.5s)
  S3
    ✓ should upload an object after tapping putObject button (2276ms)
    ✓ after tapping getObject, App should get the same object that uploaded (2224ms)
    ✓ should list Objects after tapping listObjects button (1496ms)
    ✓ should conduct multipart upload after tapping createMultipartUpload, uploadPart, completeMultipartUpload buttons (3096ms)

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.

G-Rath · 2021-03-29T17:55:23Z

@trivikr have you tested this to confirm it doesn't throw an error on import if the package isn't installed?

I would expect it to, which is why we used a try/catch over in #2108

trivikr · 2021-03-29T19:35:23Z

@trivikr have you tested this to confirm it doesn't throw an error on import if the package isn't installed?

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.

G-Rath · 2021-03-29T19:40:00Z

Ah right I see - sorry it didn't click that this change was an add, not a modify 😅

trivikr · 2021-03-29T22:36:40Z

The reason for removing require/import for react-native-get-random-values dependency is that our react-native End2End tests fail with "Unable to resolve module" error.

Screenshot

Just like uuid expects it's users to import react-native polyfills, AWS SDK for JavaScript would do the same.

trivikr · 2021-03-31T03:52:47Z

This PR is currently pending review from amplify-js team.
cc @Ashish-Nanda @amhinson

trivikr · 2021-03-31T18:17:42Z

Notes from the meeting on 3/31 with Amplify team:

Remove requirement of react-native polyfills as peerDependencies, as peerDependenciesMeta just removes the warnings but installs peerDependencies by default.
Check if a custom error can be emitted for react-native customers requesting particular polyfill as a requirement for aws-sdk just like uuid does https://github.com/uuidjs/uuid#getrandomvalues-not-supported. Amplify team had tried in the past, and it was not straight-forward.

README.md

trivikr · 2021-04-07T15:12:23Z

Remove requirement of react-native polyfills as peerDependencies, as peerDependenciesMeta just removes the warnings but installs peerDependencies by default.

Removed in 5baa663 (#2191)

Check if a custom error can be emitted for react-native customers requesting particular polyfill as a requirement for aws-sdk just like uuid does uuidjs/uuid#getrandomvalues-not-supported. Amplify team had tried in the past, and it was not straight-forward.

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 d67256e (#2191)

Related to aws#1536 Co-authored-by: Nicolas Perraut <[email protected]>

…dom-values optional peerDep

Co-authored-by: Felipe César <[email protected]>

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.

aws-sdk-js-automation · 2021-04-08T18:59:37Z

AWS CodeBuild CI Report

CodeBuild project: sdk-staging-test
Commit ID: d3fa8a1
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

iartemiev

Approved after testing the effect of this PR on Amplify RN apps.

AllanZhengYP

So nice to get rid of react-native from most users' dependencies.

No longer required aws#2191

J-Krush · 2021-04-12T14:36:33Z

So much appreciation for this PR! Unblocked on my work now :D

github-actions · 2021-04-27T00:03:20Z

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.

github-actions · 2021-10-02T00:04:16Z

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.

trivikr mentioned this pull request Mar 29, 2021

fix(middleware-retry): make react-native-get-random-values optional #2108

Closed

trivikr mentioned this pull request Mar 31, 2021

Error during the build in Netlify functions #2199

Closed

trivikr requested a review from AllanZhengYP March 31, 2021 17:30

felipecesr reviewed Apr 1, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

matrinox approved these changes Apr 4, 2021

View reviewed changes

trivikr force-pushed the optional-react-native-get-random-values branch from 31a2f8b to 5baa663 Compare April 7, 2021 15:00

This was referenced Apr 7, 2021

[@aws-sdk/middleware-retry] Drop react-native-get-random-values dependency #2192

Closed

installing v3 packages pulls in a huge number of npm packages #2219

Closed

Nicolas Perraut and others added 10 commits April 8, 2021 18:48

fix(middleware-retry): make react-native-get-random-values optional

042d75c

Related to aws#1536 Co-authored-by: Nicolas Perraut <[email protected]>

chore(middleware-sdk-transcribe-streaming): make react-native-get-ran…

eea05b1

…dom-values optional peerDep

chore: remove calls to react-native-get-random-values from code

28af36a

chore: import react-native-get-random-values in rn E2E tests

7f73246

chore(docs): update README about react-native polyfills

0908451

fix(docs): typo react-antive -> react-native

ea58362

Co-authored-by: Felipe César <[email protected]>

fix(deps): remove react-native pollyfill as a peerDependency

0ec0411

Declaring react-native polyfill in peerDependenciesMeta will just remove the warning, but will still install peerDependencies

docs(readme): add example of react-native imports

6f69c66

fix(deps): re-add react-native pollyfill as optional peerDependency

592cdaf

npm v7 does not install optional peerDependencies npm/arborist#138

fix(deps): remove react-native pollyfill as optional peerDependency

d3fa8a1

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.

trivikr force-pushed the optional-react-native-get-random-values branch from 7516038 to d3fa8a1 Compare April 8, 2021 18:48

trivikr changed the title ~~fix(deps): make react-native-get-random-values optional peerDependecy~~ fix(deps): request consumers to install react-native-get-random-values polyfill Apr 8, 2021

iartemiev approved these changes Apr 9, 2021

View reviewed changes

AllanZhengYP approved these changes Apr 9, 2021

View reviewed changes

trivikr changed the title ~~fix(deps): request consumers to install react-native-get-random-values polyfill~~ fix(deps): request consumers to install react-native polyfills Apr 9, 2021

trivikr merged commit 2d3a7f0 into aws:main Apr 9, 2021

trivikr deleted the optional-react-native-get-random-values branch April 9, 2021 16:55

trivikr added a commit to trivikr/aws-sdk-js-v3 that referenced this pull request Apr 9, 2021

chore(codegen): remove react-native-get-random-values polyfill

67a1883

No longer required aws#2191

trivikr mentioned this pull request Apr 9, 2021

Trim production dependencies #1536

Closed

coderbyheart mentioned this pull request Apr 13, 2021

Revert to npm ci based layer installation NordicSemiconductor/asset-tracker-cloud-aws-js#293

Closed

github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2021

ajredniwja mentioned this pull request Jun 3, 2021

@aws-sdk/client-s3 160mb? #2150

Closed

trivikr mentioned this pull request Jun 30, 2021

Reduce size of install #1994

Closed

aws unlocked this conversation Sep 17, 2021

github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(deps): request consumers to install react-native polyfills #2191

fix(deps): request consumers to install react-native polyfills #2191

Uh oh!

trivikr commented Mar 29, 2021 •

edited

Loading

Uh oh!

G-Rath commented Mar 29, 2021

Uh oh!

trivikr commented Mar 29, 2021

Uh oh!

G-Rath commented Mar 29, 2021

Uh oh!

trivikr commented Mar 29, 2021

Uh oh!

trivikr commented Mar 31, 2021

Uh oh!

trivikr commented Mar 31, 2021

Uh oh!

Uh oh!

trivikr commented Apr 7, 2021

Uh oh!

aws-sdk-js-automation commented Apr 8, 2021

Uh oh!

iartemiev left a comment

Uh oh!

AllanZhengYP left a comment

Uh oh!

J-Krush commented Apr 12, 2021

Uh oh!

github-actions bot commented Apr 27, 2021

Uh oh!

github-actions bot commented Oct 2, 2021

Uh oh!

Uh oh!

fix(deps): request consumers to install react-native polyfills #2191

fix(deps): request consumers to install react-native polyfills #2191

Uh oh!

Conversation

trivikr commented Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Testing

Uh oh!

G-Rath commented Mar 29, 2021

Uh oh!

trivikr commented Mar 29, 2021

Uh oh!

G-Rath commented Mar 29, 2021

Uh oh!

trivikr commented Mar 29, 2021

Uh oh!

trivikr commented Mar 31, 2021

Uh oh!

trivikr commented Mar 31, 2021

Uh oh!

Uh oh!

trivikr commented Apr 7, 2021

Uh oh!

aws-sdk-js-automation commented Apr 8, 2021

AWS CodeBuild CI Report

Uh oh!

iartemiev left a comment

Choose a reason for hiding this comment

Uh oh!

AllanZhengYP left a comment

Choose a reason for hiding this comment

Uh oh!

J-Krush commented Apr 12, 2021

Uh oh!

github-actions bot commented Apr 27, 2021

Uh oh!

github-actions bot commented Oct 2, 2021

Uh oh!

Uh oh!

trivikr commented Mar 29, 2021 •

edited

Loading