Better specify expected Node.js version and update to v20.19.2

[doshitan]

The container image was pinned to Node.js 20.18.1, see Dockerfile:

FROM node:20.18.1-bullseye-slim AS base

But attempting to use 20.18.1 to build Storybook, you encounter an error:

{
  column: 35,
  file: '/home/doshitan/projects/platform-test-nextjs/app/.storybook/main.mjs',
  length: 11,
  line: 29,
  lineText: '      nextConfigPath: path.resolve(import.meta.dirname, "../next.config.js"),',
  namespace: '',
  suggestion: ''
}
"import.meta" is not available with the "cjs" output format and will be empty
SB_CORE-SERVER_0007 (MainFileEvaluationError): Storybook couldn't evaluate your .storybook/main.mjs file.

This is due to a recent change[1] that switched to using import.meta.dirname,
a feature of Node.js ESM loading, instead of the legacy __dirname. But
automatic detection and loading a file as ESM is behind a feature flag until
Node.js v20.19.0[2]. Thus breaking the Storybook build for the project's
"official" Node 20.18.1 version and container workflow.

So:

• Update Node.js version to 20.19.2
• Add specific version requirement to package.json, which at least will
  display a warning to folks installing packages outside the container
  environment.
• Use package.json specification to load the correct Node.js version in CI

Which should help keep all the different environments in sync. And the new
Node.js module behavior is the default in later versions as well, so might as
well lean into it.

[1] navapbc/template-application-nextjs#392
[2] https://nodejs.org/en/blog/release/v20.19.0/#requireesm-is-now-enabled-by-default

Testing

Node.js 20.18.1

❯ npm run storybook-build

> app@0.1.0 storybook-build
> storybook build

@storybook/core v8.4.7

info => Cleaning outputDir: storybook-static
{
  column: 35,
  file: '/home/doshitan/projects/platform-test-nextjs/app/.storybook/main.mjs',
  length: 11,
  line: 29,
  lineText: '      nextConfigPath: path.resolve(import.meta.dirname, "../next.config.js"),',
  namespace: '',
  suggestion: ''
}
"import.meta" is not available with the "cjs" output format and will be empty
SB_CORE-SERVER_0007 (MainFileEvaluationError): Storybook couldn't evaluate your .storybook/main.mjs file.

Original error:
TypeError [ERR_INVALID_ARG_TYPE]: The "paths[0]" argument must be of type string. Received undefined
    at Object.resolve (node:path:1169:7)
    at Object.<anonymous> (/.storybook/main.mjs:29:28)
    at Module._compile (node:internal/modules/cjs/loader:1469:14)
    at Module._compile (/node_modules/esbuild-register/dist/node.js:2258:26)
    at Module._extensions..js (node:internal/modules/cjs/loader:1548:10)
    at Object.newLoader [as .mjs] (/node_modules/esbuild-register/dist/node.js:2262:9)
    at Module.load (node:internal/modules/cjs/loader:1288:32)
    at Module._load (node:internal/modules/cjs/loader:1104:12)
    at Module.require (node:internal/modules/cjs/loader:1311:19)
    at require (node:internal/modules/helpers:179:18)
    at loadMainConfig (./node_modules/@storybook/core/dist/common/index.cjs:17511:11)
    at async buildStaticStandalone (./node_modules/@storybook/core/dist/core-server/index.cjs:35381:11)
    at async withTelemetry (./node_modules/@storybook/core/dist/core-server/index.cjs:35757:12)
    at async build (./node_modules/@storybook/core/dist/cli/bin/index.cjs:2555:3)
    at async s.<anonymous> (./node_modules/@storybook/core/dist/cli/bin/index.cjs:2661:7)
{
  column: 35,
  file: '/home/doshitan/projects/platform-test-nextjs/app/.storybook/main.mjs',
  length: 11,
  line: 29,
  lineText: '      nextConfigPath: path.resolve(import.meta.dirname, "../next.config.js"),',
  namespace: '',
  suggestion: ''
}
"import.meta" is not available with the "cjs" output format and will be empty
{
  column: 35,
  file: '/home/doshitan/projects/platform-test-nextjs/app/.storybook/main.mjs',
  length: 11,
  line: 29,
  lineText: '      nextConfigPath: path.resolve(import.meta.dirname, "../next.config.js"),',
  namespace: '',
  suggestion: ''
}
"import.meta" is not available with the "cjs" output format and will be empty

Node >= 20.19.0

❯ npm run storybook-build

> app@0.1.0 storybook-build
> storybook build

@storybook/core v8.4.7

info => Cleaning outputDir: storybook-static
info => Loading presets
info => Building manager..
info => Manager built (183 ms)
info => Building preview..
info Addon-docs: using MDX3
info => Using implicit CSS loaders
info => Using SWC as compiler
info => Using default Webpack5 setup
10% building 0/1 entries 0/0 dependencies 0/0 modulesinfo => Copying static files: public at storybook-static
10% building 0/1 entries 1/1 dependencies 0/1 modulesinfo Using tsconfig paths for react-docgen

[...lots of deprecation warnings...]

info => Preview built (52 s)
info => Output directory: /home/doshitan/projects/platform-test-nextjs/app/storybook-static

The warning if running a Node.js version that doesn't match package.json requirements (in this example 20.19.0 instead of 20.19.2):

❯ npm i
npm warn EBADENGINE Unsupported engine {
npm warn EBADENGINE   package: 'app@0.1.0',
npm warn EBADENGINE   required: { node: '20.19.2', npm: '>=10.0.0' },
npm warn EBADENGINE   current: { node: 'v20.19.0', npm: '10.8.2' }
npm warn EBADENGINE }

Preview environment for app

♻️ Environment destroyed ♻️

[github-actions]

Coverage report for app

St.❔	Category	Percentage	Covered / Total
🟢	Statements	97.92%	47/48
🟢	Branches	85.71%	6/7
🟢	Functions	100%	8/8
🟢	Lines	100%	40/40

Test suite run success

5 tests passing in 3 suites.

Report generated by 🧪jest coverage report action from 24deaf7

[lorenyu]

LGTM appreciate the attention to detail. Is there any value in encapsulating the setup node step in a custom action? That way if we want to change anything such as the setup-node@v4 to @v5 we can do it one place

[doshitan]

Merged in navapbc/template-application-nextjs#399

[doshitan]

Is there any value in encapsulating the setup node step in a custom action? That way if we want to change anything such as the setup-node@v4 to @v5 we can do it one place

Hmm, possibly. There's not many spots using it today and for just version swaps, sed s// is easy enough, so doesn't feel super valuable to at the moment. Though the cache params push it to almost there.

[lorenyu]

@doshitan agree, it's kind of on the fence for me too, fine to keep as is for now