Refactor: single source of truth for RuleId, enforced at compile time#243
Merged
Conversation
The rule catalog had two independent `RuleId` types — `rules/index.ts`
derived one from `RULES_TUPLE`, `rules/rule-metadata.ts` derived one from
`RULE_DEFAULTS` — kept in agreement only by a runtime test
(`catalog.test.ts`). Worse, index's `RuleId` had silently collapsed to
`string`: factory-built rules and `: Rule`-annotated rules widen `id` to
`string`, and one such element poisons the whole `(typeof tuple)[number]["id"]`
union. So `RuleId` was a vacuous supertype and every `RuleStates` /
`RuleAvailabilityStates` map keyed by it was really `Record<string, …>`.
Make `rule-metadata.ts` the sole `RuleId` / `RULE_IDS` source and have
`index.ts` constrain its tuple against it instead of re-deriving:
- `satisfies readonly { id: RuleId }[]` — registering a rule without a
`RULE_DEFAULTS` entry is now a compile error (forward direction).
- a reverse type-level assertion naming the offender — declaring a `RuleId`
without a registered runtime is now a compile error.
Both replace runtime-only catalog checks with author-time guarantees (the
catalog test stays as belt-and-suspenders).
To make those checks non-vacuous, the literal `id` now flows through:
`createSelectorHideRule` and `defineInlineTextRedactRule` are generic over
`Id extends RuleId`; `chat-widget-hide`, `ads-hide`, and
`irrelevant-sections-redact` pin their id with `as const` / `satisfies`.
With `RuleId` now a real union, a new `CatalogRule = Rule & { id: RuleId }`
type narrows `RULES`, fixing the previously-unsound `string`-keyed indexing in
`rule-engine`, `storage`, `availability`, `lifecycle`, `RuleList`,
`parse-config`, and `page-world-hooks`.
Labels stay in `popup/rule-labels.ts`: they are the canary for
`check-background-purity.ts`, so they can't move into worker-imported metadata.
Verified: check, typecheck, 2052 tests, build (purity ok), and controlled
negative tests confirming both compile-time guards fire.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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
Collapses the rule catalog's two independent
RuleIdtypes into one, and moves catalog agreement from a runtime test to author-time compile errors.Background
Adding a rule had to keep multiple hand-maintained lists in agreement, with two independent
RuleIdtypes only reconciled bycatalog.test.ts:rules/index.tsderivedRuleIdfromRULES_TUPLErules/rule-metadata.tsderivedRuleIdfromRULE_DEFAULTSInvestigating this surfaced a worse problem: index's
RuleIdhad silently collapsed tostring. Factory-built rules (and a couple of: Rule-annotated rules) widenidtostring, and a singleid: stringelement poisons the whole(typeof RULES_TUPLE)[number]["id"]union. SoRuleIdwas a vacuous supertype, and everyRuleStates/RuleAvailabilityStatesmap keyed by it was effectivelyRecord<string, …>— the runtime test was the only thing keeping the catalog honest.What changed
rule-metadata.tsis the soleRuleId/RULE_IDSsource.index.tsre-exports both and constrains its tuple rather than re-deriving:satisfies readonly { id: RuleId }[]→ registering a rule without aRULE_DEFAULTSentry is a compile error (forward).RuleIdwithout a registered runtime is a compile error.idnow flows through so the checks aren't vacuous:createSelectorHideRuleanddefineInlineTextRedactRuleare generic overId extends RuleId;chat-widget-hide,ads-hide,irrelevant-sections-redactpin their id.CatalogRule = Rule & { id: RuleId }narrowsRULES, fixing previously-unsoundstring-keyed indexing inrule-engine,storage,availability,lifecycle,RuleList,parse-config,page-world-hooks(these were the type holesstring-RuleIdhad masked).Deliberately not done
RULE_LABELSstays inpopup/rule-labels.ts— the labels are the canary forcheck-background-purity.ts(it grepsbackground.jsfor each label string), so they can't move into worker-importedrule-metadata.ts. That duplication remains, guarded by its sync test.Verification
bun run check✓bun run typecheck✓bun run test— 2052 tests ✓bun run build✓ (background.js purity: 39 canaries, no leaks)🤖 Generated with Claude Code