plan/react-rewrite #1
@@ -0,0 +1,467 @@
|
||||
# Phase 1A-3 — ESLint Boundaries + Restricted Imports Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Add `eslint-plugin-boundaries` layered dependency rules and `no-restricted-imports` guards to the existing ESLint flat config so that architectural violations are caught at lint time — preventing features from importing routes, UI from importing features, observability from leaking SDK internals, and storage from bypassing the `src/shared/storage.ts` wrapper.
|
||||
|
||||
**Architecture:** Extends `eslint.config.js` from 1A-1 with two new plugins/rules: `eslint-plugin-boundaries` (defines element types by directory pattern, enforces which layers can import which) and built-in `no-restricted-imports` (bans specific package imports outside their designated "owner" file). Each rule has a fabricated-violation test in `tests/eslint/` that asserts the rule fires.
|
||||
|
||||
**Tech Stack:** `eslint-plugin-boundaries@^5.0.0` (ESLint 9 flat-config compatible), ESLint built-in `no-restricted-imports`.
|
||||
|
||||
**Scope:** Config-only. No runtime code, no tests of production logic. Every task is "add a rule + add a probe test that proves it fires."
|
||||
|
||||
**Prerequisites (1A-1 + 1A-2 complete):**
|
||||
- `eslint.config.js` flat config with `typescript-eslint`, `unused-imports` already configured.
|
||||
- `src/` tree with `features/`, `ui/`, `shared/`, `observability/`, `mf/`, `routes/`, `env/` directories populated.
|
||||
|
||||
---
|
||||
|
||||
## File structure
|
||||
|
||||
| File | Responsibility | Task |
|
||||
|---|---|---|
|
||||
| `eslint.config.js` | Modify: add boundaries plugin + no-restricted-imports rules | 1, 2 |
|
||||
| `tests/eslint/boundaries.test.ts` | Fabricated violation tests for boundary rules | 3 |
|
||||
| `tests/eslint/restricted-imports.test.ts` | Fabricated violation tests for restricted imports | 4 |
|
||||
|
||||
---
|
||||
|
||||
## Task 1 — Install `eslint-plugin-boundaries` and configure element types
|
||||
|
||||
**Files:**
|
||||
- Modify: `package.json` (devDep)
|
||||
- Modify: `eslint.config.js`
|
||||
|
||||
- [ ] **Step 1: Install the plugin**
|
||||
|
||||
```bash
|
||||
pnpm add -D eslint-plugin-boundaries@^5.0.0
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add boundary element types and dependency rules to `eslint.config.js`**
|
||||
|
||||
Add the following to `eslint.config.js` — import at the top, new config object appended to the array:
|
||||
|
||||
At the top of the file, add the import:
|
||||
```javascript
|
||||
import boundaries from "eslint-plugin-boundaries";
|
||||
```
|
||||
|
||||
Then add a new config object to the exported array (after the existing `files: ["src/**/*.{ts,tsx}"]` block):
|
||||
|
||||
```javascript
|
||||
{
|
||||
files: ["src/**/*.{ts,tsx}"],
|
||||
plugins: {
|
||||
boundaries,
|
||||
},
|
||||
settings: {
|
||||
"boundaries/elements": [
|
||||
{ type: "routes", pattern: "src/routes/*" },
|
||||
{ type: "mf", pattern: "src/mf/*" },
|
||||
{ type: "features", pattern: "src/features/*", capture: ["feature"] },
|
||||
{ type: "ui", pattern: "src/ui/*" },
|
||||
{ type: "shared", pattern: "src/shared/*" },
|
||||
{ type: "observability", pattern: "src/observability/*" },
|
||||
{ type: "i18n", pattern: "src/i18n/*" },
|
||||
{ type: "env", pattern: "src/env/*" },
|
||||
],
|
||||
},
|
||||
rules: {
|
||||
// Design spec §1.2 layered dependency direction:
|
||||
// features/ cannot import routes/ or mf/
|
||||
// ui/ cannot import features/
|
||||
// shared/ cannot import features/, routes/, mf/, observability/
|
||||
// observability/ cannot import features/, routes/, mf/
|
||||
"boundaries/element-types": [
|
||||
"error",
|
||||
{
|
||||
default: "allow",
|
||||
rules: [
|
||||
{
|
||||
from: "features",
|
||||
disallow: ["routes", "mf"],
|
||||
message: "Features must not import from routes/ or mf/. Use the HostContract or a shared module instead.",
|
||||
},
|
||||
{
|
||||
from: "ui",
|
||||
disallow: ["features", "routes", "mf"],
|
||||
message: "UI layer must not import from features/, routes/, or mf/. UI is consumed by features, not the other way around.",
|
||||
},
|
||||
{
|
||||
from: "shared",
|
||||
disallow: ["features", "routes", "mf", "observability"],
|
||||
message: "Shared modules must not import from features/, routes/, mf/, or observability/.",
|
||||
},
|
||||
{
|
||||
from: "observability",
|
||||
disallow: ["features", "routes", "mf"],
|
||||
message: "Observability modules must not import from features/, routes/, or mf/.",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Verify lint still passes on existing code**
|
||||
|
||||
```bash
|
||||
pnpm lint
|
||||
```
|
||||
Expected: exit 0 — existing code doesn't violate the boundaries because no cross-layer imports exist yet.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add package.json pnpm-lock.yaml eslint.config.js
|
||||
git commit -m "Add eslint-plugin-boundaries with layered dependency rules"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2 — Add `no-restricted-imports` rules
|
||||
|
||||
**Files:**
|
||||
- Modify: `eslint.config.js`
|
||||
|
||||
Four restricted-import rules per master plan §1A-3:
|
||||
1. `@opentelemetry/sdk-metrics` — only in `src/observability/metrics/otel.ts`
|
||||
2. `window.localStorage` / `window.sessionStorage` — only in `src/shared/storage.ts` (via `no-restricted-globals`)
|
||||
3. `@microsoft/signalr` — forbidden in SSR-bundle files (enforced by file-path pattern)
|
||||
4. `react-i18next` — only in `src/i18n/provider.tsx`
|
||||
|
||||
- [ ] **Step 1: Add restricted-imports rules to `eslint.config.js`**
|
||||
|
||||
Add a new config object to the array — these are file-path-scoped overrides:
|
||||
|
||||
```javascript
|
||||
// --- Restricted imports (master plan §1A-3) ---
|
||||
// OTel SDK internals: only src/observability/metrics/otel.ts may import them
|
||||
{
|
||||
files: ["src/**/*.{ts,tsx}"],
|
||||
ignores: ["src/observability/metrics/otel.ts"],
|
||||
rules: {
|
||||
"no-restricted-imports": [
|
||||
"error",
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: "@opentelemetry/sdk-metrics",
|
||||
message: "Import from @opentelemetry/api or use getMeter() from @/observability/metrics/otel instead. Direct SDK access is restricted to src/observability/metrics/otel.ts.",
|
||||
},
|
||||
{
|
||||
name: "@opentelemetry/sdk-node",
|
||||
message: "Import from @opentelemetry/api or use getMeter()/getTracer() from @/observability/metrics/otel instead. Direct SDK access is restricted to src/observability/metrics/otel.ts.",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
// react-i18next: only src/i18n/provider.tsx may import it
|
||||
{
|
||||
files: ["src/**/*.{ts,tsx}"],
|
||||
ignores: ["src/i18n/provider.tsx"],
|
||||
rules: {
|
||||
"no-restricted-imports": [
|
||||
"error",
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: "react-i18next",
|
||||
message: "Import useTranslation from @/i18n/provider instead. Direct react-i18next imports are restricted to src/i18n/provider.tsx.",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
// @microsoft/signalr: forbidden in files that run during SSR.
|
||||
// SSR-bundle = routes/ and server/ directories. Features + shared/hooks are client-side.
|
||||
{
|
||||
files: ["src/routes/**/*.{ts,tsx}", "src/server/**/*.{ts,tsx}"],
|
||||
rules: {
|
||||
"no-restricted-imports": [
|
||||
"error",
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: "@microsoft/signalr",
|
||||
message: "SignalR must not be imported in SSR-bundle files (routes/, server/). Use dynamic import in a useEffect or a client-only wrapper.",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
// window.localStorage / window.sessionStorage: only src/shared/storage.ts
|
||||
{
|
||||
files: ["src/**/*.{ts,tsx}"],
|
||||
ignores: ["src/shared/storage.ts"],
|
||||
rules: {
|
||||
"no-restricted-globals": [
|
||||
"error",
|
||||
{
|
||||
name: "localStorage",
|
||||
message: "Use the storage module from @/shared/storage instead. Direct localStorage access is restricted to src/shared/storage.ts.",
|
||||
},
|
||||
{
|
||||
name: "sessionStorage",
|
||||
message: "Use the storage module from @/shared/storage instead. Direct sessionStorage access is restricted to src/shared/storage.ts.",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Verify lint still passes**
|
||||
|
||||
```bash
|
||||
pnpm lint
|
||||
```
|
||||
Expected: exit 0 — no existing file imports these restricted packages.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add eslint.config.js
|
||||
git commit -m "Add no-restricted-imports for OTel SDK, react-i18next, SignalR SSR, localStorage"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3 — Fabricated violation tests for boundary rules
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/eslint/boundaries.test.ts`
|
||||
|
||||
These tests create temporary probe files, run `eslint` on them, and assert violations. This is NOT a vitest test — it's a shell-command test that invokes ESLint on fabricated files. But we wrap it in vitest for consistency with the test runner.
|
||||
|
||||
- [ ] **Step 1: Write `tests/eslint/boundaries.test.ts`**
|
||||
|
||||
```typescript
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { execSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dirname, "../..");
|
||||
|
||||
function lintString(filePath: string, content: string): string {
|
||||
const absPath = path.join(ROOT, filePath);
|
||||
fs.mkdirSync(path.dirname(absPath), { recursive: true });
|
||||
fs.writeFileSync(absPath, content, "utf8");
|
||||
try {
|
||||
execSync(`pnpm exec eslint "${absPath}" --no-eslintrc -c eslint.config.js --format json`, {
|
||||
cwd: ROOT,
|
||||
encoding: "utf8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
return "PASS";
|
||||
} catch (err: unknown) {
|
||||
const error = err as { stdout?: string };
|
||||
return error.stdout ?? "UNKNOWN_ERROR";
|
||||
} finally {
|
||||
fs.unlinkSync(absPath);
|
||||
}
|
||||
}
|
||||
|
||||
describe("boundaries rules", () => {
|
||||
it("features/ cannot import from routes/", () => {
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'import { something } from "../../routes/page";\nexport const x = something;\n',
|
||||
);
|
||||
expect(result).toContain("boundaries/element-types");
|
||||
});
|
||||
|
||||
it("features/ cannot import from mf/", () => {
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'import { REMOTE_BUILD_MARKER } from "../../mf/host-entry";\nexport const x = REMOTE_BUILD_MARKER;\n',
|
||||
);
|
||||
expect(result).toContain("boundaries/element-types");
|
||||
});
|
||||
|
||||
it("ui/ cannot import from features/", () => {
|
||||
const result = lintString(
|
||||
"src/ui/__test_probe__.ts",
|
||||
'import { something } from "../features/online-board";\nexport const x = something;\n',
|
||||
);
|
||||
expect(result).toContain("boundaries/element-types");
|
||||
});
|
||||
|
||||
it("shared/ cannot import from features/", () => {
|
||||
const result = lintString(
|
||||
"src/shared/__test_probe__.ts",
|
||||
'import { something } from "../features/online-board";\nexport const x = something;\n',
|
||||
);
|
||||
expect(result).toContain("boundaries/element-types");
|
||||
});
|
||||
|
||||
it("observability/ cannot import from features/", () => {
|
||||
const result = lintString(
|
||||
"src/observability/__test_probe__.ts",
|
||||
'import { something } from "../features/online-board";\nexport const x = something;\n',
|
||||
);
|
||||
expect(result).toContain("boundaries/element-types");
|
||||
});
|
||||
|
||||
it("features/ CAN import from shared/ (allowed direction)", () => {
|
||||
// This should NOT trigger a boundary violation.
|
||||
// We can't fully test this without a real shared module, so just verify
|
||||
// the rule doesn't fire on a features→env import (which is allowed).
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'import { getEnv } from "../../env";\nexport const x = getEnv;\n',
|
||||
);
|
||||
expect(result).not.toContain("boundaries/element-types");
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update vitest config to include `tests/` directory**
|
||||
|
||||
The current `vitest.config.ts` has `include: ["src/**/*.test.ts", "src/**/*.test.tsx"]`. Add `tests/` to the include:
|
||||
|
||||
Edit `vitest.config.ts` `include` to:
|
||||
|
||||
```typescript
|
||||
include: ["src/**/*.test.ts", "src/**/*.test.tsx", "tests/**/*.test.ts"],
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the boundary tests**
|
||||
|
||||
```bash
|
||||
pnpm test tests/eslint/boundaries
|
||||
```
|
||||
Expected: all 6 tests pass. The first 5 should show "boundaries/element-types" in the ESLint output; the last one should NOT.
|
||||
|
||||
If tests fail because `eslint-plugin-boundaries` doesn't detect imports via relative paths, the plugin may need the `boundaries/include` setting or file resolution config. Debug by running the lint command manually on a probe file and checking what ESLint reports.
|
||||
|
||||
If the probe tests are too slow (each spawns an ESLint process), that's acceptable — these are CI-only correctness checks, not hot-loop unit tests.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add tests/eslint/boundaries.test.ts vitest.config.ts
|
||||
git commit -m "Add fabricated violation tests for boundary rules"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4 — Fabricated violation tests for restricted imports
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/eslint/restricted-imports.test.ts`
|
||||
|
||||
- [ ] **Step 1: Write `tests/eslint/restricted-imports.test.ts`**
|
||||
|
||||
```typescript
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { execSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dirname, "../..");
|
||||
|
||||
function lintString(filePath: string, content: string): string {
|
||||
const absPath = path.join(ROOT, filePath);
|
||||
fs.mkdirSync(path.dirname(absPath), { recursive: true });
|
||||
fs.writeFileSync(absPath, content, "utf8");
|
||||
try {
|
||||
execSync(`pnpm exec eslint "${absPath}" --no-eslintrc -c eslint.config.js --format json`, {
|
||||
cwd: ROOT,
|
||||
encoding: "utf8",
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
return "PASS";
|
||||
} catch (err: unknown) {
|
||||
const error = err as { stdout?: string };
|
||||
return error.stdout ?? "UNKNOWN_ERROR";
|
||||
} finally {
|
||||
fs.unlinkSync(absPath);
|
||||
}
|
||||
}
|
||||
|
||||
describe("no-restricted-imports rules", () => {
|
||||
it("blocks @opentelemetry/sdk-metrics outside otel.ts", () => {
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'import { MeterProvider } from "@opentelemetry/sdk-metrics";\nexport const x = MeterProvider;\n',
|
||||
);
|
||||
expect(result).toContain("no-restricted-imports");
|
||||
});
|
||||
|
||||
it("blocks react-i18next outside provider.tsx", () => {
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'import { useTranslation } from "react-i18next";\nexport const x = useTranslation;\n',
|
||||
);
|
||||
expect(result).toContain("no-restricted-imports");
|
||||
});
|
||||
|
||||
it("blocks @microsoft/signalr in routes/ (SSR bundle)", () => {
|
||||
const result = lintString(
|
||||
"src/routes/__test_probe__.ts",
|
||||
'import { HubConnectionBuilder } from "@microsoft/signalr";\nexport const x = HubConnectionBuilder;\n',
|
||||
);
|
||||
expect(result).toContain("no-restricted-imports");
|
||||
});
|
||||
|
||||
it("blocks localStorage outside storage.ts", () => {
|
||||
const result = lintString(
|
||||
"src/features/online-board/__test_probe__.ts",
|
||||
'const x = localStorage.getItem("key");\nexport default x;\n',
|
||||
);
|
||||
expect(result).toContain("no-restricted-globals");
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the restricted-import tests**
|
||||
|
||||
```bash
|
||||
pnpm test tests/eslint/restricted-imports
|
||||
```
|
||||
Expected: all 4 tests pass.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add tests/eslint/restricted-imports.test.ts
|
||||
git commit -m "Add fabricated violation tests for restricted import rules"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5 — Full exit-gate verification
|
||||
|
||||
- [ ] **Step 1: Quality gates**
|
||||
|
||||
```bash
|
||||
pnpm typecheck && pnpm lint && pnpm test
|
||||
```
|
||||
Expected: typecheck exit 0, lint exit 0, all tests pass (11 existing + 10 new = 21 total).
|
||||
|
||||
- [ ] **Step 2: Verify git status**
|
||||
|
||||
```bash
|
||||
git status
|
||||
```
|
||||
Expected: clean working tree.
|
||||
|
||||
---
|
||||
|
||||
## Self-review
|
||||
|
||||
**Spec coverage.** Master plan §1A-3 exports:
|
||||
- `eslint-plugin-boundaries` rules for layered deps → Task 1
|
||||
- `no-restricted-imports`: OTel SDK, react-i18next, @microsoft/signalr SSR, localStorage → Task 2
|
||||
- Fabricated violation tests → Tasks 3, 4
|
||||
|
||||
**Placeholder scan.** No TBD/TODO. All rule configs are concrete.
|
||||
|
||||
**Type consistency.** Element type names (`routes`, `mf`, `features`, `ui`, `shared`, `observability`, `i18n`, `env`) match the `src/` directory names.
|
||||
Reference in New Issue
Block a user