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

fix(NODE-6074): Avoiding top-level awaits in bson by introducing "node" bundles #669

Closed
wants to merge 8 commits into from
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"parserOptions": {
"ecmaVersion": 2020,
"project": [
"./tsconfig.json"
"./tsconfig.common.json",
"./tsconfig.node.json"
]
},
"plugins": [
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,5 @@ docs/public
.nvmrc

benchmarks.json

*.tsbuildinfo
44 changes: 0 additions & 44 deletions etc/rollup/rollup-plugin-require-rewriter/require_rewriter.mjs

This file was deleted.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@
"exports": {
"import": {
"types": "./bson.d.ts",
"node": "./lib/bson.node.mjs",
"default": "./lib/bson.mjs"
},
"require": {
"types": "./bson.d.ts",
"node": "./lib/bson.node.cjs",
"default": "./lib/bson.cjs"
},
"react-native": "./lib/bson.rn.cjs",
Expand All @@ -107,7 +109,7 @@
"check:granular-bench": "npm run build:bench && node ./test/bench/etc/run_granular_benchmarks.js",
"check:spec-bench": "npm run build:bench && node ./test/bench/lib/spec/bsonBench.js",
"build:bench": "cd test/bench && npx tsc",
"build:ts": "node ./node_modules/typescript/bin/tsc",
"build:ts": "node ./node_modules/typescript/bin/tsc --build --force",
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding --build because the project now relies on TS project references.
Adding --force because the composite: true entails incremental: true and subsequent runs of build:dts would fail because the clean_definition_files deletes files produced by tsc and because of the incremental: true it TSC doesn't check the existence of cached files. The --force opts out of incremental builds.

"build:dts": "npm run build:ts && api-extractor run --typescript-compiler-folder node_modules/typescript --local && node etc/clean_definition_files.cjs",
"build:bundle": "rollup -c rollup.config.mjs",
"build": "npm run build:dts && npm run build:bundle",
Expand Down
32 changes: 25 additions & 7 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { nodeResolve } from '@rollup/plugin-node-resolve';
import typescript from '@rollup/plugin-typescript';
import { RequireRewriter } from './etc/rollup/rollup-plugin-require-rewriter/require_rewriter.mjs';
import { RequireVendor } from './etc/rollup/rollup-plugin-require-vendor/require_vendor.mjs';

/** @type {typescript.RollupTypescriptOptions} */
Expand All @@ -27,8 +26,17 @@ const tsConfig = {
declaration: false,
types: [],
tsconfig: false,
include: ['src/**/*']
include: ['src/**/*'],
exclude: ['src/node.ts'],
};

/** @type {typescript.RollupTypescriptOptions} */
const nodeTsConfig = {
...tsConfig,
exclude: [],
types: ['node']
};

const input = 'src/index.ts';

/** @type {import('rollup').RollupOptions} */
Expand All @@ -43,6 +51,11 @@ const config = [
exports: 'named',
sourcemap: true
},
{
file: 'lib/bson.mjs',
format: 'esm',
sourcemap: true
},
{
file: 'lib/bson.bundle.js',
format: 'iife',
Expand All @@ -54,13 +67,18 @@ const config = [
]
},
{
input,
plugins: [typescript(tsConfig), new RequireRewriter(), nodeResolve({ resolveOnly: [] })],
output: {
file: 'lib/bson.mjs',
input: 'src/node.ts',
plugins: [typescript(nodeTsConfig), nodeResolve({ resolveOnly: ['node:crypto'] })],
output: [{
file: 'lib/bson.node.mjs',
format: 'esm',
sourcemap: true
}
}, {
file: 'lib/bson.node.cjs',
format: 'commonjs',
exports: 'named',
sourcemap: true
}]
},
{
input,
Expand Down
6 changes: 6 additions & 0 deletions src/node.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { randomBytes } from 'node:crypto';

import { nodeJsByteUtils } from './utils/node_byte_utils';
nodeJsByteUtils.randomBytes = randomBytes;

export * from './index';
37 changes: 5 additions & 32 deletions src/utils/node_byte_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,6 @@ type NodeJsBufferConstructor = Omit<Uint8ArrayConstructor, 'from'> & {
// This can be nullish, but we gate the nodejs functions on being exported whether or not this exists
// Node.js global
declare const Buffer: NodeJsBufferConstructor;
declare const require: (mod: 'crypto') => { randomBytes: (byteLength: number) => Uint8Array };

/** @internal */
export function nodejsMathRandomBytes(byteLength: number) {
return nodeJsByteUtils.fromNumberArray(
Array.from({ length: byteLength }, () => Math.floor(Math.random() * 256))
);
}

/**
* @internal
* WARNING: REQUIRE WILL BE REWRITTEN
*
* This code is carefully used by require_rewriter.mjs any modifications must be reflected in the plugin.
*
* @remarks
* "crypto" is the only dependency BSON needs. This presents a problem for creating a bundle of the BSON library
* in an es module format that can be used both on the browser and in Node.js. In Node.js when BSON is imported as
* an es module, there will be no global require function defined, making the code below fallback to the much less desireable math.random bytes.
* In order to make our es module bundle work as expected on Node.js we need to change this `require()` to a dynamic import, and the dynamic
* import must be top-level awaited since es modules are async. So we rely on a custom rollup plugin to seek out the following lines of code
* and replace `require` with `await import` and the IIFE line (`nodejsRandomBytes = (() => { ... })()`) with `nodejsRandomBytes = await (async () => { ... })()`
* when generating an es module bundle.
*/
const nodejsRandomBytes: (byteLength: number) => Uint8Array = (() => {
try {
return require('crypto').randomBytes;
} catch {
return nodejsMathRandomBytes;
}
})();

/** @internal */
export const nodeJsByteUtils = {
Expand Down Expand Up @@ -162,5 +131,9 @@ export const nodeJsByteUtils = {
return nodeJsByteUtils.toLocalBufferType(buffer).write(source, byteOffset, undefined, 'utf8');
},

randomBytes: nodejsRandomBytes
randomBytes(byteLength: number) {
return nodeJsByteUtils.fromNumberArray(
Array.from({ length: byteLength }, () => Math.floor(Math.random() * 256))
);
}
};
43 changes: 43 additions & 0 deletions tsconfig.common.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"compilerOptions": {
"composite": true,
"allowJs": false,
"checkJs": false,
"strict": true,
"alwaysStrict": true,
"target": "es2021",
"module": "commonjs",
"moduleResolution": "node",
"skipLibCheck": true,
"lib": [
"es2021",
"ES2022.Error"
],
"outDir": "lib",
"importHelpers": false,
"noEmitHelpers": false,
"noEmitOnError": true,
"emitDeclarationOnly": true,
// Generate separate source maps files with sourceContent included
"sourceMap": true,
"inlineSourceMap": false,
"inlineSources": false,
// API-extractor makes use of the declarations, npm script should be cleaning these up
"declaration": true,
"declarationMap": true,
"types": [],
"isolatedModules": true,
"rootDir": "src"
},
"ts-node": {
"transpileOnly": true,
"compiler": "typescript-cached-transpile",
},
"include": [
"src/**/*"
],
"exclude": [
"src/node.ts"
]
}

38 changes: 4 additions & 34 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,37 +1,7 @@
{
"compilerOptions": {
"allowJs": false,
"checkJs": false,
"strict": true,
"alwaysStrict": true,
"target": "es2021",
"module": "commonjs",
"moduleResolution": "node",
"skipLibCheck": true,
"lib": [
"es2021",
"ES2022.Error"
],
"outDir": "lib",
"importHelpers": false,
"noEmitHelpers": false,
"noEmitOnError": true,
"emitDeclarationOnly": true,
// Generate separate source maps files with sourceContent included
"sourceMap": true,
"inlineSourceMap": false,
"inlineSources": false,
// API-extractor makes use of the declarations, npm script should be cleaning these up
"declaration": true,
"declarationMap": true,
"types": [],
"isolatedModules": true
},
"ts-node": {
"transpileOnly": true,
"compiler": "typescript-cached-transpile",
},
"include": [
"src/**/*"
"files": [],
"references": [
{ "path": "./tsconfig.common.json" },
{ "path": "./tsconfig.node.json" }
]
}
13 changes: 13 additions & 0 deletions tsconfig.node.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "./tsconfig.common.json",
"compilerOptions": {
"types": ["node"]
},
"include": [],
"files": [
"src/node.ts"
],
"references": [
{ "path": "./tsconfig.common.json" }
]
}