chore!: fix build:cjs run script, drop node 12

[shadowspawn]

The build is broken (#470), possibly by interaction of ts-transform-default-export and rollup-plugin-ts.

I tried upgrading rollup and failing to get the build working, stuck at new error:

[!] (plugin Typescript) TypeError: transformDefaultExport is not a function

However, ts-transform-default-export may no longer be required!

Installing yargs-parser (currently v21.1.1) the export in build/build.cjs looks like:

module.exports = yargsParser;

Removing ts-transform-default-export, the export in build/build.cjs looks as desired:

module.exports = yargsParser;

I see that rollup has a related configuration option, which we could perhaps set explicitly to reduce chance it changes in future: https://rollupjs.org/configuration-options/#output-exports Update: already have output.exports = 'default'.

Fixes: #470

(Opening this PR as a draft since I hacked my way to a working build without deep knowledge of rollup et al. Feedback from gentle readers on things to check whether this is an adequate solution are welcome!)

[shadowspawn]

Cough: well, that broke all the CI! Some reading to do...

[shadowspawn]

Bump the minimum node version for engine, and tests. Pinned back rollup-plugin-ts after getting errors on node 14.

Update, rollup-plugin-ts v3.2.0 dropped support for node 14 (oops!) : https://github.com/wessberg/rollup-plugin-ts/issues/202

[shadowspawn]

The official rollup typescript plugin appears to work fine (tests pass) and supports "node": ">=14.0.0", so will try switching to that.

https://www.npmjs.com/package/@rollup/plugin-typescript

[shadowspawn]

Coverage dropped slightly and failed CI check, presumably because some extra code included by different typescript plugin. I have not been brave enough to try a diff on the output yet, but now might be the time... Edit: generating identical cjs code, apart from sourceMap comment for "test" build.

Note to self: run more of the checks before pushing! I am used to editor catching the lint problems...

[shadowspawn]

Some interaction between tsconfig sourceMap and rollup sourcemap and plugin configuration meant maps were being generated. Added an explicit passthrough of the local configured setting in rollup configuration.

Edit: removed override. Generating sourcemap is expected for test due to env?

    "pretest": "rimraf build && tsc -p tsconfig.test.json && cross-env NODE_ENV=test npm run build:cjs",

[tommy-mitchell]

Pulled this branch down locally and it's working for me to compile and run tests.

Rollup supports setting environment variables through its CLI, meaning we can get rid of cross-env:

- "pretest": "rimraf build && tsc -p tsconfig.test.json && cross-env NODE_ENV=test npm run build:cjs",
+ "pretest": "rimraf build && tsc -p tsconfig.test.json && npm run build:cjs -- --environment NODE_ENV:test",

This could also be simplified to just a TEST environment variable:

// package.json
- npm run build:cjs -- --environment NODE_ENV:test
+ npm run build:cjs -- --environment TEST

// rollup.config.js
- if (process.env.NODE_ENV === 'test') output.sourcemap = true
+ if (process.env.TEST === 'true') output.sourcemap = true

[shadowspawn]

Thanks @tommy-mitchell. I will look into those suggestions.

[shadowspawn]

I like the --environment approach to drop a dependency and use the rollup support.

(The pattern of passing options into another run-script is a little unfamiliar to me, but I am coming around to the idea.)

[shadowspawn]

Reminded by the failure that the coverage fails because it is being run against the test build which adds a last line to file:

//# sourceMappingURL=index.cjs.map