porsager / prexit

A graceful way to shutdown / handle process exit
Do What The F*ck You Want To Public License
120 stars 7 forks source link

2.0 requires ESM async import() #8

Closed RoboPhred closed 1 year ago

RoboPhred commented 1 year ago

2.0 of prexit is using "export default" at the top level, making it an esm module.

This means it can no longer use typescript's import statement when typescript is using commonjs, node16, or nodeNext mode

import prexit from "prexit"
prexit(...)

This results in a nodejs error:

Error [ERR_REQUIRE_ESM]: require() of ES Module \node_modules\prexit\index.js from \backend\lib\index.js not supported.
Instead change the require of \node_modules\prexit\index.js in \backend\lib\index.js to a dynamic import() which is available in all CommonJS modules.

Instead, it now has to be imported with node's import() function, which is async

async function setupPrexit() {
  const prexit = await import("prexit");
  prexit(...);
}
setupPrexit();

Was this change intentional?

porsager commented 1 year ago

No, I don't use typescript so no idea, sorry 🙂

If you can come up with a solution that doesn't mess with the JS api, feel free to PR

coreyward commented 1 year ago

I have this issue too. Importing from postgres and other libraries is fine, but it's not possible to use prexit with module code as far as I can tell, so I either have to strip it out of my Next.js app so I can use my database config file for scripts, or find a way to load it elsewhere.

porsager commented 1 year ago

Hmm.. You're also using typescript @coreyward ?

porsager commented 1 year ago

I know others just had to turn esModuleInterop on with typescript, don't know if it's as simple as that?

coreyward commented 1 year ago

Yeah, using TS and it works fine with that, it's just that ts-node with the ESM flag doesn't like it. It's all a skosh over my head honestly.

porsager commented 1 year ago

What if you setup typescript to compile to esm like:

"compilerOptions": { "module": "ESNext",

?

Again, I'm not a typescript user, so might be way off

coreyward commented 1 year ago

In my scenario this is complicated a bit by the fact that I am using Next (where it works fine as is) but also trying to have a CLI script that can run database queries. I think I have ts-node converting TypeScript to ESM JS fine, but I think for anything in node_modules it's expecting them to still be CJS, so it's using require, maybe? If you had a CJS output available like you do for postgres it seems like it would pick that up during these runs and all would be okay. At least, that's what it seems like is happening for other dependencies where this works fine (including postgres).

For the time being, I converted prexit into TypeScript and put it in my lib folder. I had to adjust the custom event emitting because adjusting the built-in NodeJS.Signals to include "prexit" wasn't working for me (might not be viable without re-typing all of the process stuff, really), but it seems to work the same. I have no interest in maintaining a fork, especially as I don't really know what all is happening in it, but for reference if anybody else is curious:

const handlers: Record<string, Handler[] | undefined> = {}
const last: ((signal: CombinedSignal) => void)[] = []

let finished = false

export default function prexit(
  signals?: PrexitSignal[] | Handler,
  fn?: Handler
) {
  if (typeof signals === "function") {
    fn = signals
    signals = [...prexit.signals]
  }

  const combinedSignals: CombinedSignal[] = [
    "prexit",
    ...(signals ?? prexit.signals),
  ]

  let called = false
  for (const signal of combinedSignals) {
    handle(signal, (signal, error) => {
      if (called) return
      called = true
      return fn?.(signal, error)
    })
  }
}

prexit.signals = [
  "exit",
  "beforeExit",
  "uncaughtException",
  "unhandledRejection",
  "SIGTSTP",
  "SIGQUIT",
  "SIGHUP",
  "SIGTERM",
  "SIGINT",
] as const
prexit.logExceptions = true
prexit.last = addLast
prexit.exit = exit

type PrexitSignal = (typeof prexit.signals)[number]
type InternalSignal = "prexit"
type CombinedSignal = PrexitSignal | InternalSignal

type Handler = (signal: CombinedSignal, error?: unknown) => void | Promise<void>

function addLast(fn: (signal: CombinedSignal) => void) {
  if (last.length === 0) {
    prexit(() => {
      // noop
    })
  }
  last.push(fn)
}

function exit(
  signal?: number | CombinedSignal,
  code?: number,
  error?: unknown
) {
  if (typeof signal === "number") {
    error = code
    code = signal
    signal = "prexit"
  }

  if (code) process.exitCode = code
  Object.keys(handlers).length > 0
    ? processEmitWrapper("prexit", error)
    : process.exit()
}

function handle(signal: CombinedSignal, fn: Handler) {
  const handler = handlers[signal]
  if (handler) return handler.push(fn)

  const fns = (handlers[signal] = [fn])

  process.on(signal, (errorInput: unknown) => {
    void (async function () {
      const error = errorInput === signal ? null : errorInput
      if (
        (signal === "uncaughtException" || signal === "unhandledRejection") &&
        prexit.logExceptions
      )
        console.error(error)

      try {
        const xs = fns
          .map((fn) => fn(signal, error))
          .filter((x) => x && typeof x.then === "function")
        if (xs.length > 0) await Promise.all(xs)
      } catch (error) {
        if (!process.exitCode) process.exitCode = 1
        if (prexit.logExceptions) console.error(error)
      }

      done(signal, error)
    })()
  })
}

function done(signal: CombinedSignal, error?: unknown) {
  if (finished) return

  finished = true
  try {
    for (const fn of last) fn(signal)
  } catch (error_) {
    if (error) console.error(error_)
    else error = error_
  }

  process.exit()
}

function processEmitWrapper(event: string, error?: unknown) {
  if (event === "prexit") {
    const fns = handlers.prexit
    if (!fns) return
    for (const fn of fns) {
      void fn("prexit", error)
    }
  } else {
    process.emit(event as NodeJS.Signals, error as NodeJS.Signals)
  }
}
elhananjair commented 1 year ago

Hello @porsager First of all, thank you so much for such an excellent package for postgres, I had a hard time with node-postgres, things are easy in postgresjs.

I have successfully tried everything and it worked except for closing the database (session). The documentation specifies to use prexit package to do that, and I installed prexit package, but unfortunately just like the other guys have mentioned, I couldn't import the package using CommonJS way (require) while I used "require" for postgres package.

I cannot change "type": "module" in package.json since I am using other node packages that came in CommonJS format. Is there any work around to fix this?

for now, I just used process.exit()inside then

nikhilro commented 1 year ago

+1, same issue. Thanks @coreyward, will use your file for now