[Bug] ESM Custom payload converters cannot be loaded

[bergundy]

What are you really trying to do?

Use an ESM module as a custom payload converter.

Describe the bug

When an ESM module is used as a custom payload converter, the SDK tries to require it, which results in the follow error:

https://github.com/temporalio/sdk-typescript/blob/3088f86ce6fe0c7072ba55c4db901a70b631f87e/packages/common/src/internal-non-workflow/data-converter-helpers.ts#L22

(from slack)

Error [ERR_REQUIRE_ESM]: require() of ES Module *****/services/asm-saas-worker/build/lib/payload-converter.js from /Users/nohehf/Documents/ESCAPE/product/node_modules/@temporalio/common/lib/internal-non-workflow/data-converter-helpers.js not supported.
Instead change the require of payload-converter.js in *****/node_modules/@temporalio/common/lib/internal-non-workflow/data-converter-helpers.js to a dynamic import() which is available in all CommonJS modules.
    at Hook._require.Module.require (*****/node_modules/require-in-the-middle/index.js:188:39)
    at requireConverter (*****/node_modules/@temporalio/common/lib/internal-non-workflow/data-converter-helpers.js:17:18)
    at loadDataConverter (*****/node_modules/@temporalio/common/lib/internal-non-workflow/data-converter-helpers.js:46:28)
    at compileWorkerOptions (*****/node_modules/@temporalio/worker/lib/worker-options.js:167:76)
    at Worker.create (*****/node_modules/@temporalio/worker/lib/worker.js:141:75)
    at createTemporalWorker (file://*****/packages/temporal-worker/build/index.js:21:33)
    at async file://*****/services/asm-saas-worker/build/index.js:44:16 {
  code: 'ERR_REQUIRE_ESM'
}

Additional context

See the slack thread here (while the history is still retained): https://temporalio.slack.com/archives/C01DKSMU94L/p1699973888839069

[nohehf]

Thanks @bergundy for opening the issue, I'll report my findings here asap. I'm currently trying to find a way around this.

[nohehf]

Here are the things I noticed while looking through the codebase: The payload converter is loaded differently at: https://github.com/temporalio/sdk-typescript/blob/3088f86ce6fe0c7072ba55c4db901a70b631f87e/packages/common/src/internal-non-workflow/data-converter-helpers.ts#L22 and here https://github.com/temporalio/sdk-typescript/blob/3088f86ce6fe0c7072ba55c4db901a70b631f87e/packages/workflow/src/worker-interface.ts#L110

The payload converter is passed to Webpack via an alias: https://github.com/temporalio/sdk-typescript/blob/3088f86ce6fe0c7072ba55c4db901a70b631f87e/packages/worker/src/workflow/bundler.ts#L206

Whereas the other code that needs to be bundled (e.g. workflows code) is bundled in a file: https://github.com/temporalio/sdk-typescript/blob/3088f86ce6fe0c7072ba55c4db901a70b631f87e/packages/worker/src/workflow/bundler.ts#L159

This might create some issues? I don't know how Webpack will resolve the aliases here, but as I have no issue importing ESM workflows, this might be the root cause.

Also, if I run this in my development mode (importing an ESM .ts payload converter) via tsx, so maybe it would help to bundle all the code via some modern bundler like esbuild (which tsx uses) might help here (+ resolve other potential issues / add some benefits like speed).

Is there a particular reason why the bundle is in commonjs ? Just enforcing ESM seems to be the way to go nowadays. Also, node16 is not the LTS now.

PS: those are a few remarks I noted while trying to fix my issue, not critics, and I'm looking forward to helping/contributing here if possible; I hope this helps.

PS2: Also, having general guidelines / docs on how to pack our code (workflows, converters...) would be nice.

[nohehf]

After a lot of trial and error, I could finally bundle my ESM code into a commonjs payload converter using esbuild. Here's the command/settings that worked for me:

esbuild --bundle --outfile=build/index.cjs --target=esnext --platform=node --external:@temporalio/common --external:@bufbuild/protobuf src/index.ts

The key here was to externalize @temporalio dependencies from the bundle, causing issues if bundled. I also externalized the dependencies that could be imported from commonjs (here @bufbuild/protobuf).

Would you like to put this into the docs or integrate it into @temporalio/worker bundler?

[izakfilmalter]

@nohehf are you using bundleWorkflowCode? Or are you creating your own bundle? Trying to understand how to make this work with https://github.com/temporalio/samples-typescript/tree/main/production

[nohehf]

Hey @izakfilmalter We use bundleWorkflowCode (indirectly through Worker.create). We only manually bundle the converter to cjs via esbuild using the command above https://github.com/temporalio/sdk-typescript/issues/1292#issuecomment-1817586107. The converter is exported in a dedicated package and then imported using a require. So in the end we have something like:

import * as activities from './activities/index.js';
// [...]
const connection = await NativeConnection.connect({ address: url });

const require = createRequire(import.meta.url);

const worker = await Worker.create({
  workflowsPath: require.resolve('./workflows/index.js'),
  taskQueue: 'my-ts-task-queue',
  activities,
  dataConverter: {
    payloadConverterPath: require.resolve('<organization>/temporal-converter'), // <organization>/temporal-converter being the package name in our yarn/npm workspaces
  },
});

Workflows and activities are written in TS ESM next. Converter is bundled to cjs. Hope this helps :)) PS: sorry for the delay, missed the notification somehow.