jspm / jspm-cli

ES Module Package Manager
https://jspm.org
Apache License 2.0
3.79k stars 272 forks source link

conditional requirement with top-level await #2537

Open jimmywarting opened 1 year ago

jimmywarting commented 1 year ago

Hi 👋

I maintain node-domexception and for curiosity i took a look at what some CDN deliveries did to my package and how it would transform it.

it's very small script with one conditional import of an optional node:worker_threads dependency. it goes like:

if (!globalThis.DOMException) {
  try {
    const { MessageChannel } = require('worker_threads'),
    port = new MessageChannel().port1,
    ab = new ArrayBuffer()
    port.postMessage(ab, [ab, ab])
  } catch (err) {
    err.constructor.name === 'DOMException' && (
      globalThis.DOMException = err.constructor
    )
  }
}

module.exports = globalThis.DOMException

As you may notice, only if globalThis.DOMException isn't defined would it then try to do all of the other work of including worker_threads and doing all the work.

And when i tried to load this via: import('https://jspm.dev/node-domexception') then jspm felt like it also had to load additional dependencies (namely node:worker_threads, node:events): image

Which for this module is completely useless inside other environment outside of NodeJS <17

I would have wised that the code that you shipped where compiled to something more like this top level await solution instead:

if (!globalThis.DOMException) {
  try {
    const { MessageChannel } = await import('https://jspm.dev/npm:@jspm/core@2/nodelibs/worker_threads'),
    port = new MessageChannel().port1,
    ab = new ArrayBuffer()
    port.postMessage(ab, [ab, ab])
  } catch (err) {
    err.constructor.name === 'DOMException' && (
      globalThis.DOMException = err.constructor
    )
  }
}

export default globalThis.DOMException

(i'm thinking of doing this myself by converting my own package to ESM-only package and using top level await directly instead)

guybedford commented 1 year ago

The reason we don't assume top-level await for these kinds of cases is because it would mean making the module async which is not a backwards compatible change in every case.

Since you are the library author here, my suggestion would be to modify your library approach to use the "imports" pattern:

{
  "imports": {
    "#worker_threads": {
      "node": "node:worker_threads",
      "default": "./worker-threads-shell.js"
    }
  }
}

where worker-threads-shell contains export const MessageChannel = null.

Then in the application to do import { MessageChannel } from '#worker_threads' and then gate on if (MessageChannel) instead.

Do let me know if the above works for you.

jimmywarting commented 1 year ago

I have already switched to using atob solution