Close the gaps the node block leaves open - #56
Merged
Merged
Conversation
The block has not changed since it was written, and the three repositories carrying it took it at adoption with no pass ever made over their existing code. An audit found no repository that broke a rule — it found places where the block does not say enough to decide the case in front of you. A suppression read as compliance. The block forbids `any` flatly and separately allows a suppression carrying a comment, and never said whether the second reaches the first. Every consumer read that it does: 145 such suppressions, 14 in production code, none of them bare. Nine of the fourteen cite a connection handle whose type the dependency already exports, which is not a boundary where the type is unknown but a type someone chose not to write. Nothing in the block said so, so nothing stopped the fourteenth and nothing would stop the fifteenth. The suppression rule also named only the type checker's own directives, so the linter suppression every one of them actually uses fell outside a rule plainly meant to reach it. The ban on `npx` had nowhere to go. It never named what to use for a tool deliberately outside the dependency tree, and two consumers reach for `npx` in a scheduled workflow at four call sites — the outcome a prohibition with no alternative tends to produce. It was unfounded as written too: the neighbouring clause explains preferring the scripts, and that reason does not reach `npx`. What is wrong with `npx` is that an uninstalled name becomes a remote install rather than a failure. The double assertion went unmentioned, and appears 42 times in hand-written production code. It binds hand-written code only, since generated output belongs to whatever emits it and an edit there is undone on the next run. Five of the six changes match what all three consumers already do, so the release changes no consumer code. The double assertion is the one that creates follow-up work, and it creates it downstream rather than here. No rule is reversed or removed, so this is PATCH under the grading in README.md. Closes #55
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.
Closes #55.
blocks/node.mdhas not changed since it was written. The three repositories carrying it took it at adoption and no pass was ever made over their existing code, so an audit turned up not a repository that broke a rule but a set of places where the block does not say enough to decide the case in front of you.What the audit found
A suppression read as compliance. The block forbids
anyflatly and separately allows a suppression carrying a comment, and never said whether the second reaches the first. All three consumers read that it does — 145 such suppressions, 14 in production, and not one of them bare:Nine of the fourteen cite a connection handle whose type the dependency already exports. That is not a boundary where the type is unknown; it is a type someone chose not to write. Nothing in the block said so. The suppression rule also named only
@ts-ignoreand@ts-expect-error, so thebiome-ignoreevery one of the 145 actually uses fell outside a rule plainly meant to reach it.A ban with nowhere to go.
NEVER usenpx`` and nothing else — no spelling for a tool deliberately outside the dependency tree, andpnpm dlxnowhere in the block. Two consumers reach for `npx` in a scheduled workflow at four call sites. The prohibition was also unfounded as written: its neighbouring clause explains preferring the scripts, and that reason does not reach `npx`.Nothing checked at all.
as unknown as Tappears 42 times in hand-written production code, 389 counting tests, and the block says nothing about it.What changed
Six edits to
blocks/node.md, all within the two sections it already has.npxban is grounded in whatnpxdoes, and names both replacements —pnpm execfor a dependency,pnpm dlx <tool>@<version>for a one-off, at an exact version and never a dist-tagpnpm install --frozen-lockfilein CI@ts-ignoreoutrightanynames the boundaries data arrives acrossGrade
PATCH. Nothing is reversed or removed. Five of the six match what all three consumers already do, so this release changes no consumer code —
@ts-ignoreis at zero across all three,--frozen-lockfileis already universal, and the rest either ground an existing rule or settle a case the block never addressed. The double assertion is the one that creates follow-up work, and it creates it downstream.Test plan
python3 scripts/lint_blocks.py—blocks OKnpx markdownlint-cli2over the repository — 0 issuesblocks/node.mdexceeds 76 columnspnpmand Biome are named because every consumer of this block is driven by bothCHANGELOG.mdentry carries no issue or pull request referencesFollow-ups, not in this PR
One issue per repository once this releases: 13 production
anysites in one consumer and 1 in another, 42 double assertions across the three, and the fournpxcall sites. The block's remaining thinness — no testing rules, no logging rule, no error-handling section, and no requirement to name the exact lint invocation CI runs — is noted in #55 and left for its own issues.