feat(eslint-rules): add no-value-import-in-register-pages - #5
Draft
Jason Allen (JAllen2022) wants to merge 1 commit into
Draft
Conversation
Jason Allen (JAllen2022)
force-pushed
the
jallen/lint-page-object-conventions
branch
from
July 31, 2026 22:30
0a45a2f to
a6957b0
Compare
Jason Allen (JAllen2022)
changed the base branch from
main
to
jallen/eslint-rules-scaffolding
July 31, 2026 22:30
4 tasks
Jason Allen (JAllen2022)
force-pushed
the
jallen/lint-page-object-conventions
branch
2 times, most recently
from
July 31, 2026 23:43
b4d62e4 to
bccfe5c
Compare
Registration in register-pages.ts is deliberately lazy -- a () => import(...)
thunk with type-only imports -- so the barrel carries no runtime edges to the
page objects. One value import undoes that and pulls every page object into the
module graph of every flow that reaches the barrel. In a workspace with hundreds
of page objects that is the difference between loading the few a flow needs and
loading, parsing and shipping all of them.
The starter template already says "NEVER value-import a POM in this file" in a
comment. This makes it enforceable.
Two details worth review:
- The rule matches on the page directory, not on "is this import relative".
Workspaces provisioned before this package was extracted carry their own
registry and legitimately write `import { registerPage } from
"./page-registry.ts"`, and most workspaces still look like that. A naive
relative-import check flags them. pageDirectory is a rule option for layouts
that differ.
- `import { type Foo } from "../pages/Foo.ts"` is erased by TypeScript, so it is
not an offence. A side-effect import with no specifiers is.
Ships at warn rather than error: workspaces created before lazy registration
became the template may still value-import, and erroring would light up
repositories nobody has touched. Promote once that is measured.
Bumps to 0.0.3 so the platform has a published version to depend on. Per
RELEASING.md, merging the bump is the release.
Jason Allen (JAllen2022)
force-pushed
the
jallen/lint-page-object-conventions
branch
from
July 31, 2026 23:58
bccfe5c to
bf2a378
Compare
4 tasks
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.
Stacked on #6, which adds the
./eslint-rulesmechanism. Review that first. The release bump is on #7, the tip.Read #7 before forming a view on the detection logic. This PR's rule matches on a
pageDirectorypath segment. Adversarial testing found ten holes in that approach — it misses re-exports and misses sibling imports entirely when the registry lives beside its page objects, and it false-positives on npm packages whose names contain a page-ish segment. #7 replaces the detection with "is the specifier relative" plus an allowlist. That rewrite is written against #7'sdefinePomRule, so it couldn't be moved down into this PR without dragging that API with it. Happy to squash the two if you'd rather not review a superseded intermediate.Overview of Changes
Registration in
register-pages.tsis deliberately lazy — a() => import(...)thunk with type-only imports — so the barrel carries no runtime edges to the page objects. Break that and every flow reaching the barrel pulls in every page object. In a workspace with hundreds, that's the difference between loading the few a flow needs and loading, parsing and shipping all of them.The starter template already says "NEVER value-import a POM in this file" in a comment. This makes it enforceable.
Ships at
warn, noterror. Workspaces created before lazy registration became the starter template may still value-import, and erroring would light up repositories nobody has touched. Note thatwarndoes not spare the platform's AI agent, which blocks on any lint result — accepted for now.The legacy registry helper is why detection cannot simply be "is this import relative" on its own: workspaces predating
@qawolf/pomcarry their own registry and correctly writeimport { registerPage } from "./page-registry.ts". #7 keeps that case working via an allowlist.Testing
npm run tsc:check npm run lint npm test npm run build26 tests (13 new, 13 pre-existing in
pageRegistry.test.ts).Checklist