feat(frontend): a domain may not reach into another domain's innards either - #1317
Merged
Merged
Conversation
…either The deep-import rule was scoped to pages and components, so the half of ADR-001 it enforces held for everything except the domains themselves. A domain reaching past another domain's index.ts was unchecked. It cannot be one pattern, because "another domain" is relative to the file doing the importing: there is one block per domain, each naming only its own files and excusing only its own name. The list is read off the directory rather than written here, so a new domain is covered the day it exists rather than the day somebody remembers this file. The blocks do not clobber the rule above them or each other — a config block naming a rule replaces an earlier one only for files that match both, and every `files` here is disjoint from pages, from components and from every other domain. Probed all four ways round: a cross-domain deep import is refused, a domain's own innards are not, and both halves of the existing rule still fire. No allowlist entry was needed. The only two cross-domain imports in the tree, BoardMemberDialog and LineupEditor, already go through `@/domains/user`.
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 #1263.
Why
ADR-001 states two rules, and the second one — a domain is entered through its
index.ts— was enforced everywhere except inside the domains themselves. #1206 landed it scoped tosrc/pages/**andsrc/components/**, so a page reaching past a door was a red build while a domain doing the same thing was nobody's error. The one place the rule exists to protect was the one place nothing watched.That gap is also what is left of #1264, which was closed as a miscount: its "20 cross-domain imports" turned out to be domains reaching into their own innards, which the rule permits.
What this achieves
A domain's internals are free to move again. Until now that was only true against pages and components; a sibling domain could reach past the door and pin the layout behind it, and nothing would say so.
The allowlist does not grow.
yarn lint --max-warnings=0exits 0 on an unchanged tree, so this is a rule landing with no debt attached — measured onec0e11212, the 20 deep imports undersrc/domainsare every one of them a domain reading its own files, and the only two genuine cross-domain imports,domains/boards/island/BoardMemberDialog.vueanddomains/esports/island/LineupEditor.vue, already go through@/domains/user.How
It cannot be one pattern. "Another domain" is relative to the file doing the importing, and eslint's config has no way to say "any domain except the one you are in" in a single regex. So there is one block per domain:
files: ['src/domains/<d>/**']with a regex that excuses<d>by name and refuses the rest.The domain list is read off the directory with
readdirSyncrather than written out here, so a new domain is covered the day it exists rather than the day somebody remembers this file.The blocks do not clobber the rule above them, or each other. A config block naming a rule replaces an earlier block's options only for files that match both, which is the trap that let a client import through the first draft of #1183 — every
fileshere is disjoint fromsrc/pages/**, fromsrc/components/**and from every other domain.Not in scope
The
.vueexemption stays. A component is imported where it is drawn, and routing components through a barrel loads a domain's whole surface into anything that renders one — which broke two unit suites and, measured, saved nothing in the bundle. A cross-domain.vueimport is still legal, and the new block says so in a comment rather than leaving the negative lookahead to read as an oversight.Nothing is migrated here. The seven pages and thirty-five components still on
CROSSES_THE_BOUNDARYare #1257 through #1262 and #1274 through #1276.Worth a reviewer's attention
The eleven blocks are generated, so the thing to review is the regex rather than the repetition:
^@/domains/(?!<d>/)[a-z]+/(?!.*\.vue$).+. The first lookahead is what makes a domain's own innards legal; the second is the.vueexemption.One knock-on: #1266 says it deletes "whatever exception #1263 introduces". This introduces none, so that half of #1266 is already satisfied and only the shared allowlist is left for it.
Verification
yarn lint --max-warnings=0exits 0 on the branch with no allowlist change.The rule was probed all four ways round rather than assumed, each with a scratch file that was deleted afterwards:
src/domains/boardsimporting@/domains/committees/adapters/committees→ error, with the message naming the doorsrc/domains/boardsimporting@/domains/boards/adapters/boards→ allowedsrc/pagesimporting@/services/api→ still errorsrc/pagesimporting@/domains/user/adapters/user→ still errorThe last two are what confirm the new blocks did not replace the existing entry's options.
Diff breakdown —
█added░removed, scaled to the largest row.