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

Avoid importing entire crypto dependency tree if not in Node.js. #2304

Merged
merged 2 commits into from
Feb 12, 2019

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Feb 12, 2019

The apollo-server-core package uses Node's built-in crypto module only to create SHA-256 and -512 hashes.

When we're actually running in Node, the native crypto library is clearly the best way to create these hashes, not least because we can assume it will be available without having to bundle it first.

Outside of Node (such as in React Native apps), bundlers tend to fall back on the crypto-browserify polyfill, which comprises more than a hundred separate modules. Importing this polyfill at runtime (likely during application startup) takes precious time and memory, even though almost all of it is unused.

Since we only need to create SHA hashes, we can import the much smaller sha.js library in non-Node environments, which happens to be what crypto-browserify uses for SHA hashing.

The apollo-server-core package uses Node's built-in crypto module only to
create SHA-256 and -512 hashes.

When we're actually running in Node, the native crypto library is clearly
the best way to create these hashes, not least because we can assume it
will be available without having to bundle it first.

Outside of Node (such as in React Native apps), bundlers tend to fall back
on the crypto-browserify polyfill, which comprises more than a hundred
separate modules. Importing this polyfill at runtime (likely during
application startup) takes precious time and memory, even though almost
all of it is unused.

Since we only need to create SHA hashes, we can import the much smaller
sha.js library in non-Node environments, which happens to be what
crypto-browserify uses for SHA hashing, and is a widely used npm package
in its own right: https://www.npmjs.com/package/sha.js.
@benjamn benjamn force-pushed the avoid-bundling-crypto-pollyfills branch from 5f0e312 to 0875065 Compare February 12, 2019 15:49
Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@benjamn benjamn merged commit 3f7a7f3 into master Feb 12, 2019
@benjamn benjamn deleted the avoid-bundling-crypto-pollyfills branch February 13, 2019 00:10
abernix pushed a commit that referenced this pull request May 27, 2019
…#2324)

* lazy load unused lambda packages in core

* Style adjustments plus changes to account for other PRs with similar motives.

The work in #2054 was
designed in a way that, irregardless of the environment, the
`graphql-upload` package would only be loaded if uploads were enabled.
Therefore, the guard which checks `process.env.AWS_EXECUTION_ENV` should no
longer be necessary.

Additionally, we don't need to prefix our type-only variables with
underscores, as that's not a style that we've otherwise adopted.

* The work in this was mostly also implemented by #2305, #2304 and #2054, but the remaining subscriptions-transport-ws and unnecessary util.promisify imports are still super worth addressing. So, thank you!
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants