-
Notifications
You must be signed in to change notification settings - Fork 39
perf: don't bundle node:crypto in the browser bundle
#782
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
Conversation
…rypto` in the browser bundle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes the browser bundle by replacing Node.js crypto implementations with native WebCrypto API equivalents from @apify/utilities, reducing bundle size by ~2.4x. The key changes involve switching from synchronous crypto signature functions to asynchronous WebCrypto-based implementations and marking the Node.js crypto module as external in the browser bundler configuration.
- Updated signature generation functions to use async WebCrypto API implementations
- Added browser-based tests to verify cryptographic operations work correctly in secure contexts
- Upgraded
@apify/utilitiesfrom 2.18.0 to 2.23.0 to access the new async crypto functions
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/resource_clients/key_value_store.ts | Switched to async signature functions (createHmacSignatureAsync, createStorageContentSignatureAsync) with proper await usage |
| src/resource_clients/dataset.ts | Updated to use createStorageContentSignatureAsync with await |
| test/key_value_stores.test.js | Added browser tests for getRecordPublicUrl() and createKeysPublicUrl() to verify WebCrypto implementation works in browsers |
| test/datasets.test.js | Added browser tests for createItemsPublicUrl() to verify WebCrypto implementation works in browsers |
| test/_helper.js | Enhanced getInjectedPage() to navigate to URLs ensuring secure context required for Web Crypto API |
| rsbuild.config.ts | Marked crypto as external to prevent bundling Node.js crypto module in browser builds |
| package.json | Upgraded @apify/utilities dependency to version 2.23.0 |
| package-lock.json | Updated lock file to reflect new @apify/utilities version (2.23.0) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ed environments (#573) Worker-based environments (e.g. `jest` tests) do not have access to `globalThis.require()` (while calling `require()` instead works). Deeper investigation shows that `tsup` doesn't transpile this in a dangerous way, so we can use this here. Unblocks apify/apify-client-js#782 (tested) Unblocks failing tests in #570
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uses new implementations of
createStorageContentSignatureandcreateHmacSignaturefrom@apify/utilities.Those native WebCrypto API instead of importing the
node:cryptopackage, so we can markcryptoas external in the browser bundler and cut the bundle size ~2.4 times (both plain and compressed).master)perf/use-native-crypto)Note:
SubtleCryptoAPI in browsers is only available in Secure Contexts. This should be fairly safe (as anyhttps://-served page is one, as well aslocalhost).Related to #753