-
Notifications
You must be signed in to change notification settings - Fork 43
refactor [NET-1606]: Make utils package work without polyfilling
#3279
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
base: main
Are you sure you want to change the base?
refactor [NET-1606]: Make utils package work without polyfilling
#3279
Conversation
This pull request updates the `Logger` utility to make its scoping more flexible and less dependent on NodeJS `module` objects.[^1] The changes allow the logger to be initialized with either a string or an object containing an `id`, making it easier to use in different contexts beyond just files or modules. It is backward compatible. ### Changes **Logger scope improvements:** * Changed the `Logger` constructor to accept a `scope` parameter, which can be either a string or an object with an `id` property, instead of a `NodeJS.Module`. (`packages/utils/src/Logger.ts`) * Updated the `Logger.createName` static method to handle the new `scope` type, extracting the identifier appropriately whether it's a string or an object. (`packages/utils/src/Logger.ts`) * Introduced the `Scope` type alias for clarity and type safety. (`packages/utils/src/Logger.ts`) ### Future steps * `module` is a Node-specific object and is not available in ES and browser making the logger hard to use in other environments without a polyfill. Next steps would be to stop initializing `Logger` with `module`, esp. in files targetting multiple environments – subject for a separate PR. [^1]: Extracted (with changes) from #3279.
047065f to
eb389ee
Compare
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 pull request refactors the @streamr/utils package to eliminate the need for polyfills by implementing platform-specific builds for Node.js and browser environments. The refactoring introduces a sophisticated dual-build system using Rollup, TypeScript project references, and module aliasing to ensure crypto and utility functions work natively in both environments without runtime polyfilling.
Key Changes:
- Introduced platform-specific implementations for cryptographic (getSubtle) and MD5 hashing (computeMd5) functions with separate Node.js and browser versions
- Implemented a Rollup-based build system that creates separate Node.js and browser bundles with proper module aliasing
- Restructured TypeScript configuration to use project references with separate configs for Node, browser, Jest, Karma, and Rollup builds
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.browser.json (root) | Adds root-level browser TypeScript configuration with DOM lib and bundler module resolution |
| packages/utils/tsconfig.rollup.json | Configures TypeScript compilation for the Rollup configuration file |
| packages/utils/tsconfig.node.json | Defines Node.js build configuration with path mappings to Node-specific implementations |
| packages/utils/tsconfig.karma.json | Adds browser-specific path mappings for Karma test environment |
| packages/utils/tsconfig.json | Restructures to use TypeScript project references for multi-target builds |
| packages/utils/tsconfig.jest.json | Adds Node-specific path mappings and module resolution for Jest tests |
| packages/utils/tsconfig.browser.json | Defines browser build configuration with path mappings to browser-specific implementations |
| packages/utils/test/composeAbortSignals.test.ts | Updates to use type-only import syntax |
| packages/utils/test/Metric.test.ts | Updates to use type-only import syntax |
| packages/utils/src/node/md5.ts | Implements Node.js MD5 hashing using native crypto module |
| packages/utils/src/node/crypto.ts | Implements Node.js SubtleCrypto access using native webcrypto API |
| packages/utils/src/keyToArrayIndex.ts | Refactors to use @md5 alias instead of direct crypto import |
| packages/utils/src/exports.ts | Adds buffer-shim import and updates getSubtle export to use @crypto alias |
| packages/utils/src/executeSafePromise.ts | Updates Logger initialization to use string literal instead of module global |
| packages/utils/src/crossPlatformCrypto.ts | Removes generic cross-platform crypto implementation (replaced by platform-specific versions) |
| packages/utils/src/browser/os.ts | Adds browser stub for Node.js os module |
| packages/utils/src/browser/md5.ts | Implements browser MD5 hashing using md5 library |
| packages/utils/src/browser/crypto.ts | Implements browser SubtleCrypto access using Web Crypto API |
| packages/utils/src/TheGraphClient.ts | Updates Logger initialization to use string literal instead of module global |
| packages/utils/src/SigningUtil.ts | Updates to import getSubtle from @crypto alias |
| packages/utils/src/Logger.ts | Adds env() helper function for safe process.env access in browser environments |
| packages/utils/rollup.config.mts | Creates Rollup configuration for building dual Node.js and browser bundles with platform-specific aliases |
| packages/utils/package.json | Updates exports structure for dual Node/browser builds, adds new dependencies, and includes postbuild Rollup step |
| packages/utils/karma.config.ts | Adds webpack alias configuration for browser-specific module resolution |
| packages/utils/jest.config.ts | Adds module name mapping for Jest tests (contains bug - maps to browser instead of node) |
| package-lock.json | Adds dependencies for Rollup, browser polyfills (buffer, md5, path-browserify, readable-stream), and updates package versions |
| Dockerfile.node | Adds @streamr/utils to bootstrap sequence as it's now a foundational dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lotta options from there.
e23df2c to
2d9d736
Compare
This pull request refactors the
@streamr/utilspackage to provide robust, platform-specific builds for both Node.js and browser environments. It introduces dual entry points, platform-aware module aliasing, and improved type and build configurations. The changes ensure that cryptographic and utility functions work seamlessly across environments, and that the package exports are consistent and type-safe.Platform-specific build and export system:
Introduced a new
rollup.config.mtsthat builds both Node.js and browser bundles, with aliasing for platform-specific modules (e.g.,@crypto,@md5,os,path, etc.), and generates corresponding type definitions.Updated
package.jsonto use the new dual export system, specifying separate entry points and types for Node.js and browser, and adjusted the files and build scripts accordingly. [1] [2]Added
tsconfig.browser.jsonto provide TypeScript path mappings for browser-specific modules.Platform-specific implementations:
Replaced the generic
crossPlatformCryptowith dedicated implementations:src/node/crypto.tsfor Node.js andsrc/browser/crypto.tsfor browsers, both exporting agetSubtlefunction. [1] [2] [3]Provided a browser-specific
md5implementation insrc/browser/md5.tsand a Node.js version insrc/node/md5.ts, both exportingcomputeMd5. [1] [2]Added a browser stub for the
osmodule insrc/browser/os.ts.Module aliasing and test configuration:
Codebase cleanup and improvements:
Refactored imports in source files to use new platform-aware aliases for cryptography and hashing utilities, improving maintainability and clarity. [1] [2] [3] [4]
Improved environment variable access in
Logger.tsfor better browser compatibility. [1] [2] [3] [4]Miscellaneous: updated logger initialization in some files for clarity, and minor test/type import improvements. [1] [2] [3] [4]
Build process enhancements:
Added a post-build step to run Rollup after TypeScript compilation, ensuring correct bundling and type output.
Ensured all necessary dependencies for bundling and browser support are included.
Updated Dockerfile to bootstrap the
@streamr/utilspackage during the build process.