Skip to content

feat(eslint-rules): add no-value-import-in-register-pages - #5

Draft
Jason Allen (JAllen2022) wants to merge 1 commit into
jallen/eslint-rules-scaffoldingfrom
jallen/lint-page-object-conventions
Draft

feat(eslint-rules): add no-value-import-in-register-pages#5
Jason Allen (JAllen2022) wants to merge 1 commit into
jallen/eslint-rules-scaffoldingfrom
jallen/lint-page-object-conventions

Conversation

@JAllen2022

@JAllen2022 Jason Allen (JAllen2022) commented Jul 31, 2026

Copy link
Copy Markdown

Stacked on #6, which adds the ./eslint-rules mechanism. 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 pageDirectory path 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's definePomRule, 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.ts is 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, not error. Workspaces created before lazy registration became the starter template may still value-import, and erroring would light up repositories nobody has touched. Note that warn does 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/pom carry their own registry and correctly write import { 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 build

26 tests (13 new, 13 pre-existing in pageRegistry.test.ts).

Checklist

  • Changes follow the code style of this project
  • Self-review completed
  • Tests added/updated (or not applicable)
  • No breaking changes (or described below)

@JAllen2022
Jason Allen (JAllen2022) force-pushed the jallen/lint-page-object-conventions branch from 0a45a2f to a6957b0 Compare July 31, 2026 22:30
@JAllen2022 Jason Allen (JAllen2022) changed the title feat(eslint-rules): add POM lint rules and an ./eslint-rules export feat(eslint-rules): add no-value-import-in-register-pages Jul 31, 2026
@JAllen2022
Jason Allen (JAllen2022) changed the base branch from main to jallen/eslint-rules-scaffolding July 31, 2026 22:30
@JAllen2022
Jason Allen (JAllen2022) force-pushed the jallen/lint-page-object-conventions branch 2 times, most recently from b4d62e4 to bccfe5c Compare July 31, 2026 23:43
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants