|
| 1 | +/** |
| 2 | + * ObjectUI ESLint rule: no-dynamic-import-in-test-hook |
| 3 | + * |
| 4 | + * Bans `await import(…)` in a `beforeAll`/`beforeEach` body. Import the module |
| 5 | + * at module scope instead. |
| 6 | + * |
| 7 | + * A dynamic import inside a hook bills the module's cold Vite transform to |
| 8 | + * `hookTimeout`. That transform is unbounded work — under full-suite |
| 9 | + * parallelism the transform pipeline is saturated (the `dom-heavy` project |
| 10 | + * alone spends ~60s in transform) — so the hook's budget is really a bet on |
| 11 | + * machine load, and the test's reliability stops depending on the code it |
| 12 | + * covers. Imported at module scope the same cost lands in the file's import |
| 13 | + * phase, which no test or hook timeout applies to. |
| 14 | + * |
| 15 | + * This exists because raising the timeout is the intuitive "fix" and it does |
| 16 | + * not work. objectui#3010 found five load-sensitive flaky tests of this shape; |
| 17 | + * a sweep then found 36 more files, EVERY one already carrying a raised |
| 18 | + * timeout, on an escalating ladder — 15s → 30s → 60s. One of them even left |
| 19 | + * the escalation as advice (`// Increase timeout to 30 seconds for heavy |
| 20 | + * renderer imports`), and `plugin-kanban` blew its raised 15s budget anyway at |
| 21 | + * 15021ms. #3021 converted the four riskiest. The prose rule in AGENTS.md §9 |
| 22 | + * did not hold on its own, which is why this is mechanical. |
| 23 | + * |
| 24 | + * Scope, deliberately narrow to stay false-positive free: |
| 25 | + * - Only `beforeAll` / `beforeEach` as BARE identifiers. A `foo.beforeAll()` |
| 26 | + * member call is somebody else's API. |
| 27 | + * - The hook frame RESETS inside a nested function, so a lazy FACTORY — |
| 28 | + * `registerLazy('x', () => import('./x'))` — is fine: the hook registers |
| 29 | + * the loader, it does not run the import. |
| 30 | + * - A hook that also establishes per-run module state (`vi.resetModules`, |
| 31 | + * `vi.doMock`, `vi.doUnmock`, `vi.stubEnv`, `vi.stubGlobal`) is EXEMPT. The |
| 32 | + * re-import is the point there — it has to observe state that only exists |
| 33 | + * at hook time, so hoisting it would break the test rather than speed it up. |
| 34 | + * |
| 35 | + * No autofixer on purpose. The mechanical cases are a one-line move, but the |
| 36 | + * general case captures the module into a variable the tests read, and hoisting |
| 37 | + * that means introducing top-level await and reordering side effects — not a |
| 38 | + * rewrite a fixer should make unattended. |
| 39 | + * |
| 40 | + * @type {import('eslint').Rule.RuleModule} |
| 41 | + */ |
| 42 | + |
| 43 | +/** Vitest lifecycle hooks whose body is billed to `hookTimeout`. */ |
| 44 | +const HOOKS = new Set(['beforeAll', 'beforeEach']); |
| 45 | + |
| 46 | +/** |
| 47 | + * `vi.*` calls that establish per-run module state. A dynamic import in the |
| 48 | + * same hook is deliberately re-reading that state, so it cannot be hoisted. |
| 49 | + */ |
| 50 | +const MODULE_STATE_HELPERS = new Set([ |
| 51 | + 'resetModules', |
| 52 | + 'doMock', |
| 53 | + 'doUnmock', |
| 54 | + 'stubEnv', |
| 55 | + 'stubGlobal', |
| 56 | +]); |
| 57 | + |
| 58 | +/** True for `beforeAll(fn)` / `beforeEach(fn, timeout)` — bare identifier only. */ |
| 59 | +function isHookCall(node) { |
| 60 | + return ( |
| 61 | + node?.type === 'CallExpression' && |
| 62 | + node.callee.type === 'Identifier' && |
| 63 | + HOOKS.has(node.callee.name) |
| 64 | + ); |
| 65 | +} |
| 66 | + |
| 67 | +/** True for `vi.resetModules()` / `vitest.doMock(…)` and friends. */ |
| 68 | +function isModuleStateHelper(node) { |
| 69 | + const callee = node.callee; |
| 70 | + return ( |
| 71 | + callee.type === 'MemberExpression' && |
| 72 | + callee.object.type === 'Identifier' && |
| 73 | + (callee.object.name === 'vi' || callee.object.name === 'vitest') && |
| 74 | + callee.property.type === 'Identifier' && |
| 75 | + MODULE_STATE_HELPERS.has(callee.property.name) |
| 76 | + ); |
| 77 | +} |
| 78 | + |
| 79 | +export default { |
| 80 | + meta: { |
| 81 | + type: 'problem', |
| 82 | + docs: { |
| 83 | + description: |
| 84 | + 'Disallow `await import(…)` inside a beforeAll/beforeEach body — it bills an unbounded module transform to hookTimeout, making the test load-sensitive (objectui#3010/#3021).', |
| 85 | + recommended: true, |
| 86 | + }, |
| 87 | + schema: [], |
| 88 | + messages: { |
| 89 | + banned: |
| 90 | + "Do not load a module inside '{{hook}}' — the cold transform is billed to hookTimeout, so the test passes or fails on machine load. Import it at module scope instead (`import '{{source}}';`), which moves the cost to the import phase where no test or hook timeout applies. Raising the timeout is not the fix: every one of the 36 files found this way already had a raised timeout, escalating 15s -> 30s -> 60s (objectui#3010/#3021, AGENTS.md section 9). If the import must re-read per-run state, keep it and call vi.resetModules() in the same hook.", |
| 91 | + }, |
| 92 | + }, |
| 93 | + create(context) { |
| 94 | + // One frame per enclosing function. `hook` is the hook name when this |
| 95 | + // function is *directly* the hook's callback, else undefined — so the frame |
| 96 | + // resets inside a nested function and a lazy factory never reports. |
| 97 | + const frames = [{ hook: undefined, candidates: [], exempt: false }]; |
| 98 | + const top = () => frames[frames.length - 1]; |
| 99 | + |
| 100 | + return { |
| 101 | + ':function'(node) { |
| 102 | + const parent = node.parent; |
| 103 | + const hook = isHookCall(parent) && parent.arguments.includes(node) |
| 104 | + ? parent.callee.name |
| 105 | + : undefined; |
| 106 | + frames.push({ hook, candidates: [], exempt: false }); |
| 107 | + }, |
| 108 | + |
| 109 | + ':function:exit'() { |
| 110 | + const frame = frames.pop(); |
| 111 | + if (!frame.hook || frame.exempt) return; |
| 112 | + for (const { node, source } of frame.candidates) { |
| 113 | + context.report({ |
| 114 | + node, |
| 115 | + messageId: 'banned', |
| 116 | + data: { hook: frame.hook, source }, |
| 117 | + }); |
| 118 | + } |
| 119 | + }, |
| 120 | + |
| 121 | + // `import('…')` — parsers spell this `ImportExpression`. |
| 122 | + ImportExpression(node) { |
| 123 | + const frame = top(); |
| 124 | + if (!frame.hook) return; |
| 125 | + frame.candidates.push({ |
| 126 | + node, |
| 127 | + source: node.source?.type === 'Literal' ? String(node.source.value) : 'the-module', |
| 128 | + }); |
| 129 | + }, |
| 130 | + |
| 131 | + CallExpression(node) { |
| 132 | + // Mark the whole hook exempt, wherever in its body the helper sits. |
| 133 | + if (!isModuleStateHelper(node)) return; |
| 134 | + for (let i = frames.length - 1; i >= 0; i -= 1) { |
| 135 | + if (frames[i].hook) { |
| 136 | + frames[i].exempt = true; |
| 137 | + return; |
| 138 | + } |
| 139 | + } |
| 140 | + }, |
| 141 | + }; |
| 142 | + }, |
| 143 | +}; |
0 commit comments