From 023c27d37f09b52b798c643766d16853ef29042a Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Thu, 26 Jan 2023 17:51:10 -0500 Subject: [PATCH 1/2] fix(worker): Avoid webpack error on "node:" modules in workflows --- .../example/client.ts | 5 ++++- packages/test/src/test-bundler.ts | 2 +- packages/worker/src/workflow/bundler.ts | 14 ++++++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/test/src/mocks/workflows-with-node-dependencies/example/client.ts b/packages/test/src/mocks/workflows-with-node-dependencies/example/client.ts index a066e2c34..9f1604a2d 100644 --- a/packages/test/src/mocks/workflows-with-node-dependencies/example/client.ts +++ b/packages/test/src/mocks/workflows-with-node-dependencies/example/client.ts @@ -1,7 +1,10 @@ import dns from 'dns'; +import http from 'node:http'; export function exampleHeavyweightFunction(): void { + // Dummy code, only to ensure dependencies on 'dns' and 'node:http' do not get removed. + // This code will actually never run. dns.resolve('localhost', () => { - /* ignore */ + http.get('localhost'); }); } diff --git a/packages/test/src/test-bundler.ts b/packages/test/src/test-bundler.ts index 33d6094f7..70fdccfaf 100644 --- a/packages/test/src/test-bundler.ts +++ b/packages/test/src/test-bundler.ts @@ -61,7 +61,7 @@ if (RUN_INTEGRATION_TESTS) { const taskQueue = `${t.title}-${uuid4()}`; const workflowBundle = await bundleWorkflowCode({ workflowsPath: require.resolve('./mocks/workflows-with-node-dependencies/issue-516'), - ignoreModules: ['dns'], + ignoreModules: ['dns', 'http'], }); const worker = await Worker.create({ taskQueue, diff --git a/packages/worker/src/workflow/bundler.ts b/packages/worker/src/workflow/bundler.ts index 8492e5241..1fdf70188 100644 --- a/packages/worker/src/workflow/bundler.ts +++ b/packages/worker/src/workflow/bundler.ts @@ -68,7 +68,7 @@ export class WorkflowCodeBundler { this.payloadConverterPath = payloadConverterPath; this.failureConverterPath = failureConverterPath; this.workflowInterceptorModules = workflowInterceptorModules ?? defaultWorflowInterceptorModules; - this.ignoreModules = ignoreModules ?? []; + this.ignoreModules = (ignoreModules ?? []).map((x) => (x.startsWith('node:') ? x.substring('node:'.length) : x)); this.webpackConfigHook = webpackConfigHook ?? ((config) => config); } @@ -183,14 +183,20 @@ exports.importInterceptors = function importInterceptors() { entry: string, distDir: string ): Promise { - const captureProblematicModules: Configuration['externals'] = async (data, _callback): Promise => { + const captureProblematicModules: Configuration['externals'] = async ( + data, + _callback + ): Promise => { // Ignore the "node:" prefix if any. const module: string = data.request?.startsWith('node:') ? data.request.slice('node:'.length) : data.request ?? ''; - if (moduleMatches(module, disallowedModules) && !moduleMatches(module, this.ignoreModules)) { - this.foundProblematicModules.add(module); + if (moduleMatches(module, disallowedModules)) { + if (!moduleMatches(module, this.ignoreModules)) { + this.foundProblematicModules.add(module); + } + return 'var {}'; } return undefined; From 80392054cd2bfcf105d8372e3cf1d6e40ffbc696 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Fri, 27 Jan 2023 19:09:35 -0500 Subject: [PATCH 2/2] - Remove `alias` for things that `externals` can do - Add comments - Completely deny usage of the `node:` prefix, even for things modules that would otherwise be allowed --- packages/worker/src/workflow/bundler.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/worker/src/workflow/bundler.ts b/packages/worker/src/workflow/bundler.ts index 1fdf70188..d5c957f19 100644 --- a/packages/worker/src/workflow/bundler.ts +++ b/packages/worker/src/workflow/bundler.ts @@ -187,15 +187,15 @@ exports.importInterceptors = function importInterceptors() { data, _callback ): Promise => { - // Ignore the "node:" prefix if any. - const module: string = data.request?.startsWith('node:') - ? data.request.slice('node:'.length) - : data.request ?? ''; - - if (moduleMatches(module, disallowedModules)) { + const hasNodePrefix = data.request?.startsWith('node:'); + const module: string = (hasNodePrefix ? data.request?.slice('node:'.length) : data.request) ?? ''; + if (hasNodePrefix || moduleMatches(module, disallowedModules) || moduleMatches(module, this.ignoreModules)) { if (!moduleMatches(module, this.ignoreModules)) { this.foundProblematicModules.add(module); } + + // Tell webpack to replace that module by an empty object. + // This is functionnally equivalent to `alias: { 'some-module': false }`, but `externals` is called much more early return 'var {}'; } @@ -210,7 +210,6 @@ exports.importInterceptors = function importInterceptors() { alias: { __temporal_custom_payload_converter$: this.payloadConverterPath ?? false, __temporal_custom_failure_converter$: this.failureConverterPath ?? false, - ...Object.fromEntries([...this.ignoreModules, ...disallowedModules].map((m) => [m, false])), }, }, externals: captureProblematicModules,