aeharding / aeharding/wingover

imports: adopt #/ subpath imports, like Voyager

Abierto
#195 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
TypeScript
Estrellas
0
Forks
0
Merge medio
4 h 2 min
PR fusionados (30 d)
6

Descripción

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".

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.