Update node 22 - #28
Merged
Merged
Conversation
content-type@2 no longer throws on malformed Content-Type headers, so the previous try/catch became dead code (also untested). Since Flora should still reject a malformed header (unlike a valid-but-unsupported one, e.g. "text/plain", which is left untouched), the check is now done explicitly instead of relying on parse() throwing.
Node's built-in MIMEType/MIMEParams (available since Node 22, the project's minimum supported version) implements the same WHATWG media-type parsing as the content-type package, so the dependency can be dropped.
Follows the node: scheme already used for e.g. node:test/node:assert in the test suite, making all core module imports consistently distinguishable from third-party and local requires.
nicokaiser
approved these changes
Aug 12, 2026
The recursive walk() (readdir + stat per entry) is replaced by a
single fs.glob('*/**/{config.*,index.js}') call, which handles the
recursion and file-type filtering natively. Since walk() had only
one call site and is now just a short loop, it's inlined into
configLoader instead of kept as a separate function.
Config parsing is now done inline as each config.* file is discovered by the fs.glob loop, instead of in a separate Promise.all pass over the collected resources afterwards. This drops the intermediate configFile bookkeeping property, at the cost of parsing configs sequentially instead of concurrently. Errors are now raised as ImplementationError (consistent with the rest of lib/), wrapping the original parser error via `cause` instead of rewriting its message.
Loading a resource's index.js is now done inline as it's discovered by the fs.glob loop, instead of in a separate Promise.all pass. Since require() is synchronous, the Promise wrapper that pass used is no longer needed, and the intermediate instanceFile bookkeeping property can be dropped along with it.
Replace the access().then(() => true).catch(() => false) boolean dance with a plain try/catch around fs.access(). Also throw ImplementationError instead of a plain Error, consistent with the rest of the file, and rephrase the message to "Cannot access ..." since access() can fail for reasons other than the directory not existing (e.g. missing permissions); the original error is attached as cause.
Replaces the hand-rolled POJO mock with stream.Readable-based fixtures so tests exercise the actual Node stream interface instead of a reimplementation of it, and adds coverage for bodies delivered in multiple chunks and across real async ticks.
POST tests never reused the beforeEach-provided GET request — they overrode method, headers and body anyway, so reassigning the shared httpRequest variable only obscured that each test builds its own independent request.
The httpRequest.on('error', ...) handler had no test, despite guarding
against a real crash risk: an unhandled 'error' event on an
EventEmitter throws. Destroys the request stream while it's still
being read to verify the rejection message.
Replace the four scattered clearTimeout calls with a single
`httpRequest.once('close', ...)` listener, since 'close' fires exactly
once regardless of how the request concludes (success, abort, or
error). Add tests asserting clearTimeout is actually invoked on the
success, error, and aborted paths.
'error', 'aborted', and 'end' each occur at most once per request, so make that explicit instead of using on().
forEach can't stop iterating once a duplicate parameter is found, so it kept checking the remaining parameters unnecessarily. A for-of loop can return immediately once rejecting.
Same rationale as the GET query parameter loop: forEach kept checking remaining keys after a duplicate was already found and rejected.
filter().filter().forEach() always scanned all child nodes and threw from inside forEach; find() stops at the first match and the throw moves into a plain if-block.
mbaumgartl
force-pushed
the
update-node-22
branch
from
August 17, 2026 18:29
18d85e7 to
9c8f441
Compare
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.
Summary
Raises the minimum supported Node.js version to 22 and does the cleanup that unlocks / follows from it.
engines.nodebumped to>=22;ecmaVersionineslint.config.jsbumped to2023accordingly.Object.prototype.hasOwnProperty.call(...)occurrences withObject.hasOwn().no-useless-assignmentviolations this surfaced.@xmldom/xmldom,chokidar(4 → 5),luxon,serve-static(1 → 2),globals,jsdoc,prettier.content-typedependency.lib/url-parser.jsnow parsesContent-Typeheaders with Node's built-innode:utilMIMEType, which throws on malformed input — no third-party parser needed.require()s are now prefixed withnode:.lib/config-loader.js: replaced the manual recursive directory walk withfs.glob(); config parsing and resource loading now happen inline while walking. Note:fs.glob()silently skips unreadable directories instead of throwing, unlike the oldreaddir/statwalk — worth a second look if that matters for your setup.