pinojs / thread-stream

A streaming way to send data to a Node.js Worker Thread
MIT License
229 stars 23 forks source link

Error in TypeScript + ESM setup #112

Open Frando opened 1 year ago

Frando commented 1 year ago

Hi, I'm using thread-stream (through pino) in a project that uses TypeScript compiled to ESM modules, running in Node.js.

I'm getting this error when running the project:

file:///path-to-project/node_modules/thread-stream/index.js:50
  const toExecute = bundlerOverrides['thread-stream-worker'] || join(__dirname, 'lib', 'worker.js')
                                                                     ^
ReferenceError: __dirname is not defined in ES module scope
This file is being treated as an ES module because it has a '.js' file extension and '/path-to-project/packages/repco-cli/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at createWorker (file:///path-to-project/node_modules/thread-stream/index.js:50:70)
    at new ThreadStream (file:///path-to-project/node_modules/thread-stream/index.js:224:19)
    at buildStream (file:///path-to-project/node_modules/pino/lib/transport.js:21:18)
    at transport (file:///path-to-project/node_modules/pino/lib/transport.js:110:10)
    at normalizeArgs (file:///path-to-project/node_modules/pino/lib/tools.js:297:16)
    at pino (file:///path-to-project/node_modules/pino/pino.js:86:28)
    at file:///path-to-project/packages/repco-common/src/log.ts:29:20
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at ESMLoader.import (node:internal/modules/esm/loader:526:24)

I could fix the error through a hackish solution for the time being:

import module from 'module'
import p from 'path'
import pino from 'pino'

// Fix thread-stream error due to __dirname
// @ts-ignore
globalThis.__bundlerPathsOverrides = {
  'thread-stream-worker': p.join(
    p.dirname(module.createRequire(import.meta.url).resolve('thread-stream')),
    'lib',
    'worker.js',
  ),
}

const logger = pino()

However, it would be nice if that could be fixed somehow upstream.

mcollina commented 1 year ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

izakfilmalter commented 8 months ago

@Frando You should check out https://github.com/wd-David/esbuild-plugin-pino. Solved my issues for me.

claudioc commented 7 months ago

I tried looking into a way to make dirname always available but it's not trivial, since ideally one should check the presence of "import.meta" and if it's there, then use the fileUrlToPath method to define dirname.

Unfortunately nodejs fails with an un-catcheable syntax error if you try to just "typeof import.meta" when you are in commonjs even in a code branch that's not supposed to run...

Not sure how to do that. Luckily as @Frando pointed out - thanks to the good design one can override the 'thread-stream-worker' and everything works

mcollina commented 7 months ago

I would love for a better solution šŸ˜­. Each thread must have a different entry point.

claudioc commented 7 months ago

@mcollina can you please introduce pre-processor instructions to Nodejs? :D

ifdef ESM

const __dirname = ...

endif

Done! ;)

Seriously though, do you know why testing for the availability of "import.meta" generates an uncatchable error in node (it's a syntax error)? Tried in node 20...

Anyway, there should also be a way to NOT use __dirname altogether here.

albinekb commented 6 months ago

This is my workaround šŸ™ƒ

const dirname =
  typeof __dirname !== 'undefined'
    ? __dirname
    : fileURLToPath(new URL('.', import.meta.url))

I wish there was a better way...