Fix ffbt CI, engines and webpack watch typing so rc.4 can build and publish - #225
Merged
Merged
Conversation
webpack-dev-server 5 requires Node >= 18.12.0 and open@10 requires Node >= 18,
so the previous `engines: {node: ">=12"}` was no longer truthful and the CI
matrix (12.x/14.x/16.x) could not install the dependency tree at all.
- engines.node: ">=12" -> ">=18.12.0"
- Node CI matrix: [12.x, 14.x, 16.x] -> [18.x, 20.x, 22.x]
- Publish workflow: Node 14 -> 22
Both workflows now install with --legacy-peer-deps. That works around a
pre-existing conflict unrelated to this change: the root pins tslint ^6.0.0
while tslint-config-airbnb@5.11.2 peer-requires tslint ^5.11.0. npm 6 ignored
peer conflicts, but npm >= 7 treats this as fatal, which is why `npm i` fails
on every currently supported Node and why CI is red on master today. Working
around it here keeps this PR landable and unblocks the publish workflow;
untangling the TSLint stack properly belongs in its own change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Compiler.watch()` is typed `undefined | Watching` in current webpack 5
releases (it was `Watching` back in 5.75), so dereferencing it in the cleanup
callback fails to compile:
src/services/webpack/runner/run-compile-watcher.ts(23,13):
error TS18048: 'watcher' is possibly 'undefined'.
This is pre-existing and not specific to the dev-server migration — the file is
byte-identical to master. It stayed hidden because this repo sets
`package-lock=false`, so a fresh install drifts webpack from 5.75 to 5.110
within the declared `^5.75.0`, and because CI could not get past `npm i` to
reach the build at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Follow-up to #224. That PR merged at its first commit, so
masternow has the webpack-dev-server v5 migration (wds ^5.2.6,typescript ^4.9.5, version1.0.0-rc.4) without the two commits that make it build and publish. Master is currently in a state where:engines.nodestill claims>=12, but wds 5 requires>= 18.12.0andopen@10requires>= 18[12.x, 14.x, 16.x]matrix — none of which can run this dependency treenpm-publish.ymlstill runs Node 14 with a plainnpm iCompiler.watch()is dereferenced unguarded, which does not compile against current webpack 5The publish workflow will fail as master stands.
npm publishfiresprepack→npm run build, which dies on:So rc.4 cannot be released until this lands.
What
Exactly the two commits missing from #224:
Move CI and engines to Node 18+ for webpack-dev-server v5engines.node:>=12→>=18.12.0(truthful for wds 5 / open 10)[12.x, 14.x, 16.x]→[18.x, 20.x, 22.x]--legacy-peer-deps, working around a pre-existing conflict unrelated to this work: the root pinstslint ^6.0.0whiletslint-config-airbnb@5.11.2peer-requirestslint ^5.11.0. npm 6 ignored peer conflicts; npm ≥ 7 treats it as fatal, which is whynpm ifails on every currently supported Node and why CI has been red on master. Untangling the TSLint stack properly deserves its own change.Guard Compiler.watch() result, which is optional in current webpack 5Compiler.watch()is typedundefined | Watchingin current webpack 5 (it wasWatchingin 5.75), so the cleanup callback needswatcher?.close(...).package-lock=false, so a fresh install drifts webpack from 5.75 to 5.110 inside the declared^5.75.0, and because CI never got pastnpm ito reach the build.Verification
Node CI green on this branch across the full new matrix:
Locally, with a fresh
npm i --legacy-peer-deps(webpack 5.110.0, TypeScript 4.9.5):npm run buildexit 0,npm run test:ci2 suites / 19 tests passed.Consumer smoke test (
peo-frontend, vianpm pack+ tarball install):npm startboots the dev server on :9095 with no schema error, webpack compiles successfully, and middleware precedence holds —/dev-api/documentsreturnsapplication/jsonand/api/v2/edit/setDatesreturnstext/plain OK, i.e. handled by the app's own routes rather than swallowed byhistoryApiFallback.After this merges
rc.4 can be released (create a GitHub release →
npm-publish.ymlpublishes withsecrets.NPM_AUTH_TOKEN). Consumers on wds 5 then needonBeforeSetupMiddleware→setupMiddlewares, as described in #224.🤖 Generated with Claude Code