From bc7db2b22851981986380ba3033560e3f7a764e4 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 30 Nov 2021 15:02:57 -0800 Subject: [PATCH 1/3] Hardcode grpc version --- packages/firestore/rollup.config.js | 34 ++++++------------- .../src/platform/node/grpc_connection.ts | 12 ++----- 2 files changed, 13 insertions(+), 33 deletions(-) diff --git a/packages/firestore/rollup.config.js b/packages/firestore/rollup.config.js index a75701466ad..214dd6328ac 100644 --- a/packages/firestore/rollup.config.js +++ b/packages/firestore/rollup.config.js @@ -15,35 +15,21 @@ * limitations under the License. */ -import tmp from 'tmp'; -import json from '@rollup/plugin-json'; +import { version as grpcVersion } from '@grpc/grpc-js/package.json'; import alias from '@rollup/plugin-alias'; +import json from '@rollup/plugin-json'; +import copy from 'rollup-plugin-copy'; +import { terser } from 'rollup-plugin-terser'; import typescriptPlugin from 'rollup-plugin-typescript2'; +import tmp from 'tmp'; import typescript from 'typescript'; import replace from 'rollup-plugin-replace'; -import copy from 'rollup-plugin-copy'; -import { terser } from 'rollup-plugin-terser'; -import pkg from './package.json'; import { generateBuildTargetReplaceConfig } from '../../scripts/build/rollup_replace_build_target'; -const util = require('./rollup.shared'); +import pkg from './package.json'; -// Customize how import.meta.url is polyfilled in cjs nodejs build. We use it to be able to use require() in esm. -// It only generates the nodejs version of the polyfill, as opposed to the default polyfill which -// supports both browser and nodejs. The browser support is unnecessary and doesn't work well with Jest. See https://github.com/firebase/firebase-js-sdk/issues/5687 -function importMetaUrlPolyfillPlugin() { - return { - name: 'import-meta-url-current-module', - resolveImportMeta(property, { moduleId }) { - if (property === 'url') { - // copied from rollup output - return `new (require('url').URL)('file:' + __filename).href`; - } - return null; - } - }; -} +const util = require('./rollup.shared'); const nodePlugins = function () { return [ @@ -69,7 +55,8 @@ const nodePlugins = function () { ] }), replace({ - 'process.env.FIRESTORE_PROTO_ROOT': JSON.stringify('src/protos') + 'process.env.FIRESTORE_PROTO_ROOT': JSON.stringify('src/protos'), + '__GRPC_VERSION__': grpcVersion }) ]; }; @@ -120,8 +107,7 @@ const allBuilds = [ }, plugins: [ ...util.es2017ToEs5Plugins(/* mangled= */ false), - replace(generateBuildTargetReplaceConfig('cjs', 2017)), - importMetaUrlPolyfillPlugin() + replace(generateBuildTargetReplaceConfig('cjs', 2017)) ], external: util.resolveNodeExterns, treeshake: { diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index f025e38a4eb..068b7342678 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -15,10 +15,6 @@ * limitations under the License. */ -// This is a hack fix for Node ES modules to use `require`. -// @ts-ignore To avoid using `allowSyntheticDefaultImports` flag. -import module from 'module'; - import { Metadata, GrpcObject, @@ -38,11 +34,9 @@ import { logError, logDebug, logWarn } from '../../util/log'; import { NodeCallback, nodePromise } from '../../util/node_api'; import { Deferred } from '../../util/promise'; -// This is a hack fix for Node ES modules to use `require`. -// @ts-ignore To avoid using `--module es2020` flag. -const requireInESM = module.createRequire(import.meta.url); -// eslint-disable-next-line @typescript-eslint/no-require-imports -const { version: grpcVersion } = requireInESM('@grpc/grpc-js/package.json'); +// TODO: Fetch runtime version from grpc-js/package.json instead +// when it becomes possible to use require() in Node ESM without hacks. +const grpcVersion = '__GRPC_VERSION__'; const LOG_TAG = 'Connection'; const X_GOOG_API_CLIENT_VALUE = `gl-node/${process.versions.node} fire/${SDK_VERSION} grpc/${grpcVersion}`; From ff265d5fe2a01f17b0f83423cea72d219b9cec1d Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 30 Nov 2021 15:58:28 -0800 Subject: [PATCH 2/3] Restore import.meta --- packages/firestore/rollup.config.js | 21 +++++++++++++++++-- .../src/platform/node/grpc_connection.ts | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/firestore/rollup.config.js b/packages/firestore/rollup.config.js index 214dd6328ac..6e0638284cb 100644 --- a/packages/firestore/rollup.config.js +++ b/packages/firestore/rollup.config.js @@ -19,11 +19,11 @@ import { version as grpcVersion } from '@grpc/grpc-js/package.json'; import alias from '@rollup/plugin-alias'; import json from '@rollup/plugin-json'; import copy from 'rollup-plugin-copy'; +import replace from 'rollup-plugin-replace'; import { terser } from 'rollup-plugin-terser'; import typescriptPlugin from 'rollup-plugin-typescript2'; import tmp from 'tmp'; import typescript from 'typescript'; -import replace from 'rollup-plugin-replace'; import { generateBuildTargetReplaceConfig } from '../../scripts/build/rollup_replace_build_target'; @@ -31,6 +31,22 @@ import pkg from './package.json'; const util = require('./rollup.shared'); +// Customize how import.meta.url is polyfilled in cjs nodejs build. We use it to be able to use require() in esm. +// It only generates the nodejs version of the polyfill, as opposed to the default polyfill which +// supports both browser and nodejs. The browser support is unnecessary and doesn't work well with Jest. See https://github.com/firebase/firebase-js-sdk/issues/5687 +function importMetaUrlPolyfillPlugin() { + return { + name: 'import-meta-url-current-module', + resolveImportMeta(property, { moduleId }) { + if (property === 'url') { + // copied from rollup output + return `new (require('url').URL)('file:' + __filename).href`; + } + return null; + } + }; +} + const nodePlugins = function () { return [ typescriptPlugin({ @@ -107,7 +123,8 @@ const allBuilds = [ }, plugins: [ ...util.es2017ToEs5Plugins(/* mangled= */ false), - replace(generateBuildTargetReplaceConfig('cjs', 2017)) + replace(generateBuildTargetReplaceConfig('cjs', 2017)), + importMetaUrlPolyfillPlugin() ], external: util.resolveNodeExterns, treeshake: { diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 068b7342678..f7d21bd4670 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -35,7 +35,7 @@ import { NodeCallback, nodePromise } from '../../util/node_api'; import { Deferred } from '../../util/promise'; // TODO: Fetch runtime version from grpc-js/package.json instead -// when it becomes possible to use require() in Node ESM without hacks. +// when there's a cleaner way to dynamic require in both Node ESM and CJS const grpcVersion = '__GRPC_VERSION__'; const LOG_TAG = 'Connection'; From 303c5128686a23674c6eb7e2954ea6d51597e95f Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 30 Nov 2021 16:12:20 -0800 Subject: [PATCH 3/3] update comment --- packages/firestore/src/platform/node/grpc_connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index f7d21bd4670..651b436724b 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -35,7 +35,7 @@ import { NodeCallback, nodePromise } from '../../util/node_api'; import { Deferred } from '../../util/promise'; // TODO: Fetch runtime version from grpc-js/package.json instead -// when there's a cleaner way to dynamic require in both Node ESM and CJS +// when there's a cleaner way to dynamic require JSON in both Node ESM and CJS const grpcVersion = '__GRPC_VERSION__'; const LOG_TAG = 'Connection';