fix: Use cjs extension for diagnose output

[mrkjdy]

I ran into a problem using the diagnostics because my package.json type is set to module. To get the diagnostics script to run I needed to rename the output file to use the cjs extension instead of js. This PR fixes that and a typo

[badeball]

Can you explain to me what's going on here and why this change is necessary? I'm assuming this has something to do with some internal workings of esbuild.

Also, any change should ideally be accompanied by a / some tests which illustrates the problem and make sure that it doesn't resurface again.

[mrkjdy]

First, you should understand how Node determines if a file is CommonJS or an ES Module. See https://nodejs.org/docs/latest-v18.x/api/packages.html#determining-module-system for more info on how Node determines which to use.

Since the output script is placed into the workspace of the project that invoked the diagnose script, Node will consider the file as either a CommonJS script or an ES Module based on how that project is configured. If the project has it's package.json type set to "module", then Node will throw an error when the output file is require()d by diagnose.ts on line 161, because an ES Module cannot be required. To make Node consider the output file as an CommonJS module, it's extension just needs to be renamed to cjs.

Here's an example of that error:

Another option is to use a dynamic import on line 161 instead of require. This may be the more logical option as esbuild will output a IIFE meant for the browser by default, so the output file isn't really CommonJS. Dynamic imports are supported from Node 13 and up in both CommonJS and ES Modules. I'm not sure which versions of Node you are targeting, so it may be a breaking change to use a dynamic import. It looks like you're already targeting a version of Node that supports dynamic imports so I can change it to use one instead.

Which would you prefer? I'll create tests as well

[badeball]

I don't have a strong preference towards any of the solutions. It's more important to me that there's sufficient test coverage.

[mrkjdy]

I tried using a dynamic import, but it unfortunately doesn't work because TypeScript will convert it into a require() unless the module is set to ES in the compiler options. Changing the output module type won't work either because the code is require()d by node_modules/@cucumber/cucumber/lib/api/support.js and ESM modules can't be required.

I was able to add a test confirming that the change is working. It fails if you revert it to .js

[badeball]

I've merged and released this as v16.0.2.