Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): resolve webcrypto from node:crypto for Node18 #13599

Merged
merged 7 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
24 changes: 16 additions & 8 deletions .github/actions/load-verdaccio-with-amplify-js/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ runs:
steps:
- name: Start verdaccio
run: |
npx verdaccio@5.25.0 &
# This version supports Node.js v22
npx verdaccio@5.31.1 &
while ! nc -z localhost 4873; do
echo "Verdaccio not running yet"
sleep 1
Expand All @@ -18,25 +19,32 @@ runs:
- name: Install and run npm-cli-login
shell: bash
env:
NPM_REGISTRY: http://localhost:4873/
NPM_REGISTRY_HOST: localhost:4873
NPM_REGISTRY: http://localhost:4873
NPM_USER: verdaccio
NPM_PASS: verdaccio
NPM_EMAIL: verdaccio@amplify.js
run: |
npm i -g npm-cli-adduser
npm-cli-adduser
sleep 1
# Make the HTTP request that npm addUser makes to avoid the "Exit handler never called" error
TOKEN=$(curl -s \
-H "Accept: application/json" \
-H "Content-Type:application/json" \
-X PUT --data "{\"name\": \"$NPM_USER\", \"password\": \"$NPM_PASS\", \"email\": \"$NPM_EMAIL\"}" \
$NPM_REGISTRY/-/user/org.couchdb.user:$NPM_USER 2>&1 | jq -r '.token')
# Set the Verdaccio registry and set the token for logging in
yarn config set registry $NPM_REGISTRY
npm set registry $NPM_REGISTRY
npm set //"$NPM_REGISTRY_HOST"/:_authToken $TOKEN
- name: Configure registry and git
HuiSF marked this conversation as resolved.
Show resolved Hide resolved
shell: bash
working-directory: ./amplify-js
env:
NPM_REGISTRY: http://localhost:4873/
NPM_REGISTRY: http://localhost:4873
NPM_USER: verdaccio
NPM_PASS: verdaccio
NPM_EMAIL: verdaccio@amplify.js
run: |
yarn config set registry $NPM_REGISTRY
npm set registry $NPM_REGISTRY
git config --global user.email $NPM_EMAIL
git config --global user.name $NPM_USER
git status
Expand Down
6 changes: 4 additions & 2 deletions .github/actions/node-and-build/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ inputs:
is-prebuild:
required: false
default: false
node_version:
required: false
runs:
using: 'composite'
steps:
- name: Setup Node.js 18
- name: Setup Node.js
uses: actions/setup-node@b39b52d1213e96004bfcb1c61a8a6fa8ab84f3e8 # v4.0.1
with:
node-version: 18.20.2
node-version: ${{ inputs.node_version || '18.x' }}
env:
SEGMENT_DOWNLOAD_TIMEOUT_MINS: 2
- uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4.0.0
Expand Down
9 changes: 8 additions & 1 deletion .github/integ-config/integ-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ tests:
sample_name: [guest-access]
spec: storage-client-server
browser: *minimal_browser_list

# INAPPMESSAGING
- test_name: integ_in_app_messaging
desc: 'React InApp Messaging'
Expand All @@ -856,3 +856,10 @@ tests:
spec: ssr-context-isolation
yarn_script: ci:ssr-context-isolation
browser: [chrome]
- test_name: integ_node_envs
desc: 'Node.js environment tests'
framework: node
category: integration
sample_name: auth-gql-storage
yarn_script: ci:node-env-test
node_versions: ['18.x', '20.x', '22.x']
7 changes: 7 additions & 0 deletions .github/workflows/callable-e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ on:
yarn_script:
required: false
type: string
node_versions:
required: false
type: string

env:
AMPLIFY_DIR: /home/runner/work/amplify-js/amplify-js/amplify-js
Expand All @@ -54,6 +57,8 @@ jobs:
- ${{ fromJson(inputs.browser) }}
sample_name:
- ${{ fromJson(inputs.sample_name) }}
node_version:
- ${{ fromJson(inputs.node_versions) }}
fail-fast: false
timeout-minutes: ${{ inputs.timeout_minutes }}

Expand All @@ -64,6 +69,8 @@ jobs:
path: amplify-js
- name: Setup node and build the repository
uses: ./amplify-js/.github/actions/node-and-build
with:
node_version: ${{ matrix.node_version }}
- name: Setup samples staging repository
uses: ./amplify-js/.github/actions/setup-samples-staging
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/callable-e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ jobs:
timeout_minutes: ${{ matrix.integ-config.timeout_minutes || 35 }}
retry_count: ${{ matrix.integ-config.retry_count || 3 }}
yarn_script: ${{ matrix.integ-config.yarn_script || '' }}
node_versions: ${{ toJSON(matrix.integ-config.node_versions) || '[""]' }}

# e2e-test-runner-headless:
# name: E2E test runnner_headless
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@
"webpack-bundle-analyzer": "^4.7.0",
"webpack-cli": "^5.0.0"
},
"engines": {
"node": ">=18"
},
"resolutions": {
"@types/babel__traverse": "7.20.0",
"path-scurry": "1.10.0",
Expand Down
9 changes: 6 additions & 3 deletions packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@
"devDependencies": {
"typescript": "5.0.2"
},
"engines": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sanity check: Any risk of breakage here? What happens in NPM & yarn if someone is using an older version?

Copy link
Member Author

Choose a reason for hiding this comment

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

As adding the node version restriction is considered as a breaking change, removing it from this PR. When the library is used in a Node.js environment with version < 16, an error will be thrown as unable to resolve the Web crypto API.

"node": ">=18"
},
"size-limit": [
{
"name": "[Analytics] record (Pinpoint)",
Expand Down Expand Up @@ -383,7 +386,7 @@
"name": "[Auth] confirmSignIn (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ confirmSignIn }",
"limit": "28.27 kB"
"limit": "28.30 kB"
},
{
"name": "[Auth] updateMFAPreference (Cognito)",
Expand Down Expand Up @@ -449,13 +452,13 @@
"name": "[Auth] Basic Auth Flow (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ signIn, signOut, fetchAuthSession, confirmSignIn }",
"limit": "30.06 kB"
"limit": "30.10 kB"
},
{
"name": "[Auth] OAuth Auth Flow (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ signInWithRedirect, signOut, fetchAuthSession }",
"limit": "21.47 kB"
"limit": "21.50 kB"
},
{
"name": "[Storage] copy (S3)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ describe('getGlobal', () => {

expect(getCrypto()).toEqual(mockCrypto);
});

it('should throw error if crypto is unavailable globally', () => {
mockWindow.mockImplementation(() => undefined);

expect(() => getCrypto()).toThrow(AmplifyError);
});
});

describe('getBtoa()', () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/utils/globalHelpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ export const getCrypto = () => {
return crypto;
}

try {
const crypto = require('node:crypto').webcrypto;

if (typeof crypto === 'object') {
return crypto;
}
} catch (_) {
// no-op
}

throw new AmplifyError({
name: 'MissingPolyfill',
message: 'Cannot resolve the `crypto` function from the environment.',
Expand Down
Loading