Add test workflow

[jcbhmr]

BLOCKED by #3 since it uses npm test in CI

This PR would...

• add a test.yml action
• run tests against Node.js v16 v18 and v20

UNDRAFT this when/if #3 is merged

[jimmywarting]

Hmm, mostly hope/rely on that NodeJS have tested DOMException against WPT themself so i don't have to run any test on my own.

When NodeJS stop supporting v16 and v18 becomes LTS then i will probably mark this a legacy and deprecate the NPM package.

This package was really only meant for running inside of NodeJS v10.5.x - 16.x.x.
I wanted to build this as a replacement to the original domexception library with a smaller footprint and who also matched the exact same class instance as NodeJS so me and others could do err instanceof DOMException checks and ofc to also throw own constructed error messages
This also ment that i could not or did not want to use any ESM syntax or top level await. cuz that would have menat that i could not run this in node v10 and ppl would not be able to change their own dependency easily.

in other words i wanted it to be more of a polyfill rather than a ponyfill that had it's own class instance.

Any way. it feels pointless to run the test on NodeJS v17+ when there is a built in global DOMException available
But if we are going to run any WPT test then maybe we could do what i have done in fetch-blob where i run wpt test in nodejs with a custom experimental http-loader

but now instead do it with the built in --experimental-network-imports flag instead

WPT stuff can change ___.any.js to ___any.worker-module.js to get them as ESM instead

If we are using http import then we don't have to creating any git submodule or anything complicated (their repo is really large). Then we could always use the newest and latest test cases, and i wouldn't mind if they added something new that suddenly wasn't covered by us. it would just let me know that there is something that needs fixing or i could otherwise just disable some certain rules.

import 'https://wpt.live/webidl/ecmascript-binding/es-exceptions/DOMException-constants.any.worker-module.js'
import 'https://wpt.live/webidl/ecmascript-binding/es-exceptions/DOMException-constructor-and-prototype.any.worker-module.js'
import 'https://wpt.live/webidl/ecmascript-binding/es-exceptions/DOMException-constructor-behavior.any.worker-module.js'
import 'https://wpt.live/webidl/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.worker-module.js'

but it requires some bit more of a pre-work to set up so it can run in NodeJS. eg you need to add globalThis.self = globalThis

But if we are limiting ourself to use experimental-network-imports then it's only going to work on v17.6.0+, v16.15.0+

The engine i currently support is 10.5+

  "engines": {
    "node": ">=10.5.0"
  },

[jimmywarting]

Anyhow, adding test cases with files & folders, github workflow, and a package lock file when there isn't any dependencies feels like over doing it.

Feel like a good recommendation for ppl would be if they did do:

globalThis.DOMException || require('node-domexception')
globalThis.DOMException || await import('node-domexception')

[jcbhmr]

When NodeJS stop supporting v16 and v18 becomes LTS then i will probably mark this a legacy and deprecate the NPM package.

👍

This package was really only meant for running inside of NodeJS v10.5.x - 16.x.x.

Would it be prudent to add an export condition to block all other runtimes? or no? like:

  "exports": {
    "deno": "./DNE",
    "bun": "./DNE",
    "node": "./index.js",
    "default": "./DNE"
  },

https://github.com/jcbhmr/node-45981

This would hard-fail when bundling with something like Vite since it would be unable to find the file instead of giving a runtime error/no-op or whatever later after successfully building. I think compile-time failures like that are better than runtime "nodejsthing is not defined" errors. Don't think the errors thing is applicable here, but the I like the idea of a no-op for browsers #6

The engine i currently support is 10.5+

as for the exports suggestion, don't worry you can do main and exports and the main is a fallback if nodejs doesn't understand what exports means (like old v10)

👍 I can make it test v10 v12 v14 and v16 if that sounds good?

---

I was interested in incorporating this node-domexception polyfill into a Minimum Common Web Platform https://common-min-api.proposal.wintercg.org/ bundle polyfill that doesn't seem to exist yet on google
https://github.com/jcbhmr/common-min-api is my idea...

To do this, I'd wanted to make it as "pseudo-universal" as possible and kinda wanted to do these things to this package. Of them, the tests were the first thing that seemed easiest:

1. Add export conditions to no-op in non-Node.js environments Avoid including when bundling #6
2. Add a test to make sure the global is applied
3. Add .d.ts files so that it's "correct" even if browser DOMException types change (Constructing DOMException with causes whatwg/webidl#969) ?
4. Improve readme to add more explainer on when and why and how to use this polyfill
5. Add an isomorphic export path like node-domexception/DOMException.js or something that doesn't globalThis.DOMException = DOMException but just imports it?
6. Possibly add a shim userland DOMException class? This could make Node.js <v10 supported maybe? but I'm not sure this is relevant in this day & age. This might also be beneficial for early versions of Bun or other non-browser JS runtimes like QuickJS or others

Basically make it more universal than just a Node.js drop-in.

[jimmywarting]

I was interested in incorporating this node-domexception polyfill into a Minimum Common Web Platform https://common-min-api.proposal.wintercg.org/ bundle polyfill

That sounds interesting. if you are only going to support NodeJS v16.7+ then i may suggest a alternative solution that i have found in the undici package. it works similar to my package (Uses web api that throws DOM error and catches it)

it dose not involve any built in built in NodeJS specific node modules like using worker_thread so i consider this to be more cross env. friendlier way of obtaining the DOMException class with less code and it can function the exact same way in any env.

const DOMException = globalThis.DOMException || (() => { 
  try { atob(1) } catch (err) { return err.constructor } 
})()

// or

globalThis.DOMException ??= (() => { 
  try { atob(1) } catch (err) { return err.constructor } 
})()

i think this is so small that you could just as well inline it without using any dependencies.
atob became available in NodeJS v16+ globally

My solution is for something that works in much earlier NodeJS versions.

And the reason i think you should only support node v16.7+ is so that you can depend on the undici package to bring in their fetch impl into common-min-api polyfill

[jimmywarting]

Hmm, i had some 2nd toughs on what i want to do right now.

i might just want to do something like:

export default globalThis.DOMException ??= (() => { 
  try { atob(1) } catch (err) { return err.constructor } 
})()

Create a

"version": "2.x.x",
"type" "module",
"engines": {
    "node": ">=16.0.0"
  },

And then change the readme to say something in terms of:
If you need older support then you could install v1 instead. it supports cjs while the v2 only supports ESM.
alternatively just remove export default and only have

globalThis.DOMException ??= (() => { 
  try { atob(1) } catch (err) { return err.constructor } 
})()

And just don't export anything. then this could work with both import and require()

[jimmywarting]

I had a 2nd look at what some CDN deliveries did to my package:

esm.sh

import DOMException from 'https://esm.sh/[email protected]'

/* esm.sh - [email protected] */
export * from "https://esm.sh/v124/[email protected]/es2022/node-domexception.mjs";
export { default } from "https://esm.sh/v124/[email protected]/es2022/node-domexception.mjs";

/* esm.sh - esbuild bundle([email protected]) es2022 production */
import __1$ from "/error.js?type=unsupported-nodejs-builtin-module&name=worker_threads&importer=node-domexception";var x=Object.create;var c=Object.defineProperty;var d=Object.getOwnPropertyDescriptor;var M=Object.getOwnPropertyNames;var _=Object.getPrototypeOf,g=Object.prototype.hasOwnProperty;var h=(e=>typeof require<"u"?require:typeof Proxy<"u"?new Proxy(e,{get:(o,t)=>(typeof require<"u"?require:o)[t]}):e)(function(e){if(typeof require<"u")return require.apply(this,arguments);throw new Error('Dynamic require of "'+e+'" is not supported')});var m=(e,o)=>()=>(o||e((o={exports:{}}).exports,o),o.exports),b=(e,o)=>{for(var t in o)c(e,t,{get:o[t],enumerable:!0})},a=(e,o,t,l)=>{if(o&&typeof o=="object"||typeof o=="function")for(let s of M(o))!g.call(e,s)&&s!==t&&c(e,s,{get:()=>o[s],enumerable:!(l=d(o,s))||l.enumerable});return e},n=(e,o,t)=>(a(e,o,"default"),t&&a(t,o,"default")),p=(e,o,t)=>(t=e!=null?x(_(e)):{},a(o||!e||!e.__esModule?c(t,"default",{value:e,enumerable:!0}):t,e));var i=m((w,f)=>{if(!globalThis.DOMException)try{let{MessageChannel:e}=__1$,o=new e().port1,t=new ArrayBuffer;o.postMessage(t,[t,t])}catch(e){e.constructor.name==="DOMException"&&(globalThis.DOMException=e.constructor)}f.exports=globalThis.DOMException});var r={};b(r,{default:()=>O});var D=p(i());n(r,p(i()));var{default:u,...E}=D,O=u!==void 0?u:E;export{O as default};
/*! Bundled license information:

node-domexception/index.js:
  (*! node-domexception. MIT License. Jimmy Wärting <https://jimmy.warting.se/opensource> *)
*/
//# sourceMappingURL=node-domexception.mjs.map

it resulted in a error due to it trying to load:
https://esm.sh/error.js?type=unsupported-nodejs-builtin-module&name=worker_threads&importer=node-domexception

// Which contained this code:
throw new Error("[esm.sh] " + "unsupported nodejs builtin module \"worker_threads\" (Imported by \"node-domexception\")");
export default null;

esm.sh failed to see worker_threads as an optional dependency that isn't always needed. and it should not have tried to load worker_threads in the first place...

jsdeliver

jsdeliver dose a okey job when doing

import DOMException from 'https://cdn.jsdelivr.net/npm/[email protected]/+esm'

the require() syntax isn't converted to import and it dose not need have to call require() or import worker_threads at all.

jspm.dev

Oh man... this one have tried shipping a browserified version of worker_threads into the browser by using real web workers.
but i don't know why it felt like it had to import the optional worker_threads when it didn't have to load a optional dependency. i would have hope that it converted my code into something like:

/*! node-domexception. MIT License. Jimmy Wärting <https://jimmy.warting.se/opensource> */

if (!globalThis.DOMException) {
  try {
    const { MessageChannel } = await import('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 kind of wish that every CDN delivery did this kind of conversion and used top-level await instead...

unpkg.com

loading node-domexception would fail b/c it would not do any conversion. And then throw on module.exports is not defined

[jimmywarting]

My conclusion

i think if i did release this package as a true ESM-only package and i did this conditional top level await import of worker_thread myself then the CDN might not feel like it had to convert cjs to esm in a "badly" generated garbage output.

either way this could be solved very gracefully if i don't depend on worker_threads at all. and just used the atob hack instead.

[jimmywarting]

@jcbhmr what do you think i should do?

Use the globalThis.atob solution and require NodeJS v16+ ?

Should this be converted to a ESM only module by adding { "type": "module" } and be required to use import?

Or should it still be possible to use cjs/require? ditching the export default and not having anything exported would mean that one can choose to either use import 'node-domexception' or require('node-domexception') and the DOMException would be added onto the globalThis and essentually making this a polyfill only package (then there is nothing that is exported)

[jcbhmr]

@jcbhmr what do you think i should do?

Use the globalThis.atob solution and require NodeJS v16+ ?

Should this be converted to a ESM only module by adding { "type": "module" } and be required to use import?

Or should it still be possible to use cjs/require? ditching the export default and not having anything exported would mean that one can choose to either use import 'node-domexception' or require('node-domexception') and the DOMException would be added onto the globalThis and essentually making this a polyfill only package (then there is nothing that is exported)

I think that ESM is the way forward as cited by the famous sindre https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c
But I do know that there are still projects that are CJS-only and need CJS dependencies (since CJS can't require ESM, but ESM can import CJS) 🤷‍♂️
IMO the ESM is worth it since it's the modern way to do stuff. But that's me.

As for the atob() solution, I think that it's a great solution! Supporting LTS-only versions of Node.js is a good move IMO. I like using modern stuff, but I understand that if you have legitimate consumers of this package using old Node.js versions that this could be a bad tradeoff to make and sticking with something older (like the current worker_threads postMessage()) might be better.

As for the polyfill-only package vs the other ponyfill stuff, I think that comes down to your preference since you're the package author. I know I like to try and make polyfills "composable" into libraries if possible so that a lib author can use import fetch from "fetch-polyfill/fetch.js" or similar and NOT impact the global scope while also re-using the polyfill's code/architecture.

But since this is such aa tiny polyfill that might not be needed.

In summary:

1. Make it ESM
2. Good find on atob()! I'd use that
3. Either make it:
   • polyfill-only
   • separate files, one for polyfill, one for ponyfill

[jimmywarting]

separate files, one for polyfill, one for ponyfill

That seems good to me.
doing a polyfill only would also mean that it dose not have to be a ESM only package. cjs author could then do:

require('node-domexception/polyfill.js')

where as ESM author could use either:

import 'node-domexception/polyfill.js' // or
import { DOMException } from 'node-domexception/ponyfill.js'

The main file could still be pointed to the old legacy worker_thread solution and the new atob solution could be the new recommended way forward that only lives in either polyfill.js or ponyfill.js
this two new files would only work for NodeJS v16+

where as the main legacy worker_thread solution dose not change at all. and still supports v10.5