aeharding / aeharding/wingover

imports: adopt #/ subpath imports, like Voyager

Aperta
#195 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
TypeScript
Stelle
0
Fork
0
Merge medio
4h 2m
PR unite (30g)
6

Descrizione

Relative imports get worse the deeper the tree goes, and the three-bucket split
made them deeper: `../../../shared/map/config` is now a normal sight, and every
file move rewrites a pile of specifiers for no semantic reason.

Voyager solves it and is worth copying more or less verbatim.

## How Voyager actually does it

Two pieces, and neither is an eslint plugin — worth stating plainly because it
is easy to assume otherwise:

**1. Node subpath imports** in `package.json`:

```json
"imports": { "#/*": "./src/*" }
```

Native resolution, no bundler alias, no `tsconfig` `paths`. Vite honours it and
TypeScript honours it under `moduleResolution: "bundler"` — which is what this
repo already sets (`tsconfig.json:6`), so nothing has to change there.

**2. The built-in `no-restricted-imports`**, no plugin:

```js
patterns: [
{
regex: "\\.\\.\\/\\w+\\/",
message: "Import via absolute path (e.g. #/helpers/myHelper)",
},
],
```

Note what that regex actually bans: `../someDir/` — reaching UP and then ACROSS.
It deliberately still allows `./sibling` and `../sibling`. That is the right
line: a sibling import is legible, a `../../../` chain is not. Voyager runs it
at `"warn"`; we would probably want `"error"`, since `pnpm lint` is
`--max-warnings 0` here anyway.

## The catch, and it is the whole reason this needs care

Three checks in this repo resolve ONLY relative specifiers, and would be
silently bypassed by every `#/` import:

- `eslint-rules/ui-bucket-isolation.js:85` — `if (!spec.startsWith(".")) return`
- `eslint-rules/default-import-name.js:53` — same guard
- `scripts/check-css-conventions.mjs:106,170` — `resolve(dirname(f), from)`

So adopting `#/` without touching these would quietly reopen everything #189
just closed: `import X from "#/ui/flight/FlightSurface"` inside `src/ui/app`
would pass the bucket rule, and the app/flight wall would be enforcement in
name only. That is not hypothetical — it is the same shape as the four holes an
adversarial review already found in that rule.

**Any PR doing this must teach all three to resolve `#/*` -> `src/*` first**, and
prove it with probes the way the last round did.

## Also worth checking before committing to it

- The existing seam lints (`NO_MAP_BACKEND_MODULE`, `NO_IONIC`, the headless
scopes in `eslint.config.js`) match on SPECIFIER STRINGS with globs. Those
patterns were written against relative paths; some will need a `#/` twin, and
#182 already burned a round on a group pattern that matched nothing.
- `vitest` resolution (it shares Vite's, so likely free) and the `vi.mock`
specifiers in the unit tests.
- `screenshots/*.mjs` and `e2e/*` are outside `src/`, so they keep relative
imports either way.
- Whether the CSS side gets an equivalent. `@value` / `composes` specifiers are
resolved by postcss, not by Node, so `#/` will NOT work there without a
resolver — the stylesheets would stay relative, which is a wrinkle worth
deciding on deliberately rather than discovering.

## Suggested shape

1. Add the `imports` field; leave every existing import alone.
2. Teach the two custom rules and the CSS checker about `#/`, with probes.
3. Turn on the `no-restricted-imports` pattern as an error.
4. Codemod the existing `../../` chains in one mechanical commit, so review can
read it as "paths only".

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.