From a74e4c6fc46d85e612a576ce1a88e4e732471c94 Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 2 Aug 2026 12:23:10 -0600 Subject: [PATCH] fix: break the import cycle that put useCalculators in the TDZ The dashboard threw on arrival from any other page: ReferenceError: Cannot access 'useCalculators' before initialization at Module.useCalculators at DashboardCalculators `Calculators.js` exports `useCalculators` as a `const` and imports the ten calculators it renders. Five of those reached back up to `Apps.js` for `ColoredInput`, and `Apps.js` imports `CalculatorsPage` from `Calculators.js` -- so the hub was in a cycle with its own leaves: Calculators.js -> calculators/ElectricalCalculators.js -> Apps.js -> Calculators.js Webpack compiles a `const` export to a live getter, and a getter read before the module body reaches the declaration THROWS rather than returning undefined. That is the whole mechanism behind this particular error, and it names the const it could not reach. `ColoredInput` is a text field with a Label welded to one edge -- the calculators' unit markers. It has no business in the module that ROUTES to the calculators, so it moved to `components/ui/Inputs.tsx`. Nothing in `components/ui` imports from `components/`, so a control kept there cannot close a loop like this again. Gallery entry added, per the standing rule. `src/import-cycles.test.js` is the guard: a small import-graph walker, scoped to hubs that export a `const` into their own subtree. It fails with the cycle printed, and it self-checks against a known cycle so it cannot pass by finding nothing. Verified red with the cycle planted back. HONEST LIMIT: the guard is static because I could not reproduce the RUNTIME error in a test. TDZ needs a module to be mid-evaluation when the getter is read, and by the time any spec body runs every module has finished; babel also papers over it, handing back undefined where webpack throws. I tried an e2e that navigates to the dashboard and collects window errors -- it passes with the cycle present, so it would have been a test that cannot fail for the reason it claims, and I deleted it rather than ship it. A react-refresh trigger did not reproduce it either. What is checkable, and what is genuinely the defect, is the cycle. 1080 jest / 60 suites, 158 in ui-layout.cy.js, clean build. Co-Authored-By: Claude Opus 5 (1M context) --- app/src/components/Apps.js | 16 +-- app/src/components/ThemeSamplePage.js | 11 ++ .../components/calculators/DriveCalculator.js | 3 +- .../calculators/ElectricalCalculators.js | 3 +- .../calculators/FoodStorageCalculator.js | 3 +- .../components/calculators/HamCalculators.js | 3 +- .../calculators/RationCalculator.js | 3 +- app/src/components/ui/Inputs.tsx | 50 +++++++ app/src/import-cycles.test.js | 125 ++++++++++++++++++ 9 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 app/src/import-cycles.test.js diff --git a/app/src/components/Apps.js b/app/src/components/Apps.js index d7f1093c..724ae665 100644 --- a/app/src/components/Apps.js +++ b/app/src/components/Apps.js @@ -9,7 +9,7 @@ import { toLocaleString, useTitle } from "./Common"; -import {Header, Label, Loading, Panel, Statistic, StatisticGroup, TextInput} from "./ui"; +import {Header, Loading, Panel, Statistic, StatisticGroup} from "./ui"; import {useStatistics} from "../hooks/customHooks"; import {CalculatorsPage} from "./Calculators"; @@ -107,17 +107,3 @@ export function MoreRoute(props) { } - -/** - * A text input with a colored tag attached to its leading (or trailing) edge. - * Replaces Semantic's `} labelPosition=.../>`. - */ -export function ColoredInput({name, value, label, color, labelPosition = 'left', fluid, style, ...props}) { - const labelNode = label ? : null; - - return
- {labelNode && labelPosition === 'left' && labelNode} - - {labelNode && labelPosition !== 'left' && labelNode} -
-} diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index 0ccb38e5..731c7d1a 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -5,6 +5,7 @@ import { Button, Card, CardGroup, + ColoredInput, Confirm, Header, Icon, @@ -572,6 +573,16 @@ export function ThemeSamplePage() { action={} /> +
+ + + +
+

+ ColoredInput welds a Label to one edge of a field — the calculators' unit + markers. In night and amber the Label is an outline rather than a fill, so + the marker reads without a block of colour. +

setComments(e.currentTarget.checked)}/> diff --git a/app/src/components/calculators/DriveCalculator.js b/app/src/components/calculators/DriveCalculator.js index ea9fcc29..c9ca3fbd 100644 --- a/app/src/components/calculators/DriveCalculator.js +++ b/app/src/components/calculators/DriveCalculator.js @@ -1,6 +1,5 @@ import React from "react"; -import {Button, ButtonGroup, Grid, Group, Header, IconButton, NumberInput, Panel, Table} from "../ui"; -import {ColoredInput} from "../Apps"; +import {Button, ButtonGroup, ColoredInput, Grid, Group, Header, IconButton, NumberInput, Panel, Table} from "../ui"; import {InfoPopup, roundDigits, Toggle, useLocalStorage} from "../Common"; import {Media} from "../../contexts/contexts"; import {IconSettings, IconSettings2} from "@tabler/icons-react"; diff --git a/app/src/components/calculators/ElectricalCalculators.js b/app/src/components/calculators/ElectricalCalculators.js index 0adf483b..92233f1f 100644 --- a/app/src/components/calculators/ElectricalCalculators.js +++ b/app/src/components/calculators/ElectricalCalculators.js @@ -1,7 +1,6 @@ import {InfoPopup, roundDigits, Toggle, useLocalStorage, useLocalStorageInt} from "../Common"; import React, {useState} from "react"; -import {Grid, Header, Message, Panel, Select, Stack, Table} from "../ui"; -import {ColoredInput} from "../Apps"; +import {ColoredInput, Grid, Header, Message, Panel, Select, Stack, Table} from "../ui"; import {Media} from "../../contexts/contexts"; const initialState = { diff --git a/app/src/components/calculators/FoodStorageCalculator.js b/app/src/components/calculators/FoodStorageCalculator.js index 471bbf59..3c42a807 100644 --- a/app/src/components/calculators/FoodStorageCalculator.js +++ b/app/src/components/calculators/FoodStorageCalculator.js @@ -1,6 +1,5 @@ import React from "react"; -import {Grid, Header, Panel, Table, TextInput, Toggle} from "../ui"; -import {ColoredInput} from "../Apps"; +import {ColoredInput, Grid, Header, Panel, Table, TextInput, Toggle} from "../ui"; import {InfoPopup, roundDigits, useLocalStorage} from "../Common"; // Generic food-storage planner: given a household, estimate how much of each food category to store for a chosen diff --git a/app/src/components/calculators/HamCalculators.js b/app/src/components/calculators/HamCalculators.js index 65ddb937..0459f7e1 100644 --- a/app/src/components/calculators/HamCalculators.js +++ b/app/src/components/calculators/HamCalculators.js @@ -1,6 +1,5 @@ import React from "react"; -import {Grid, Header, Table} from "../ui"; -import {ColoredInput} from "../Apps"; +import {ColoredInput, Grid, Header, Table} from "../ui"; import {roundDigits} from "../Common"; // --------------------------------------------------------------------------- diff --git a/app/src/components/calculators/RationCalculator.js b/app/src/components/calculators/RationCalculator.js index 9991bf92..c34bb4fe 100644 --- a/app/src/components/calculators/RationCalculator.js +++ b/app/src/components/calculators/RationCalculator.js @@ -1,6 +1,5 @@ import React from "react"; -import {Button, Grid, Header, Table} from "../ui"; -import {ColoredInput} from "../Apps"; +import {Button, ColoredInput, Grid, Header, Table} from "../ui"; import {InfoPopup, roundDigits, useLocalStorage} from "../Common"; import {formatDuration} from "./WaterCalculator"; import {findNameKey, planSupplyPurchase} from "../inventory/summarize"; diff --git a/app/src/components/ui/Inputs.tsx b/app/src/components/ui/Inputs.tsx index ea0f9808..4490d90a 100644 --- a/app/src/components/ui/Inputs.tsx +++ b/app/src/components/ui/Inputs.tsx @@ -9,6 +9,7 @@ import { Textarea as MTextarea, TextInput as MTextInput, } from '@mantine/core'; +import {Label, LabelProps} from './Feedback'; /* * Form controls. @@ -165,6 +166,55 @@ export const ActionInput = forwardRef(( }); ActionInput.displayName = 'ActionInput'; +export interface ColoredInputProps + extends Omit, 'color' | 'style'> { + /** Shown as a Label beside the field. Omit it and this is a plain TextInput. */ + label?: React.ReactNode; + /* + * Applied to the ROW, not the field — every call site uses it for width or margins + * around the pair. Narrowed from Mantine's own `style`, which is a union that also + * accepts a function and an array and so cannot be spread into an object literal. + */ + style?: React.CSSProperties; + /* + * A token colour name for the label, and the SAME union Label accepts rather than a + * loose `string`: the point of the union is that a theme must have a token by that + * name, and widening it here would let a call site name a colour that resolves to + * nothing in every theme. + */ + color?: LabelProps['color']; + labelPosition?: 'left' | 'right'; + /** Fill the row rather than sizing to content. */ + fluid?: boolean; +} + +/** + * A text field with a Label welded to one side — the calculators' unit markers ("ft", "Ω", + * "gal"), which is where all ten of them get their shape. + * + * It lives here rather than beside the calculators because it used to live in `Apps.js`, + * the module that ROUTES to the calculators. Five calculators imported it, so + * `Calculators.js` -> a calculator -> `Apps.js` -> `Calculators.js` was a cycle around the + * hub, and `useCalculators` is a `const`: webpack compiles that to a getter that throws + * rather than returning undefined when read too early. The dashboard threw on arrival for + * a whole editing session. Nothing in `components/ui` imports from `components/`, so a + * control kept here cannot close a loop like that again. + */ +export function ColoredInput({ + name, value, label, color, labelPosition = 'left', fluid, style, ...props +}: ColoredInputProps) { + const labelNode = label ? : null; + + return
+ {labelNode && labelPosition === 'left' && labelNode} + + {labelNode && labelPosition !== 'left' && labelNode} +
+} + export { MTextInput as TextInput, MTextarea as Textarea, diff --git a/app/src/import-cycles.test.js b/app/src/import-cycles.test.js new file mode 100644 index 00000000..343406bc --- /dev/null +++ b/app/src/import-cycles.test.js @@ -0,0 +1,125 @@ +import fs from 'fs'; +import path from 'path'; + +/* + * Import cycles that a `const` export cannot survive. + * + * `Calculators.js` exports `useCalculators` as a `const` arrow function and imports the ten + * calculators it renders. Five of those reached back up to `Apps.js` for `ColoredInput`, + * and `Apps.js` imports `CalculatorsPage` from `Calculators.js` -- so the hub was in a cycle + * with its own leaves. + * + * Webpack compiles a `const` export to a live getter, and a getter read before the module + * body reaches the declaration throws `ReferenceError: Cannot access 'useCalculators' before + * initialization` rather than returning undefined. With every module evaluated once, in + * order, the cycle is harmless; it stops being harmless the moment anything re-executes a + * module in the middle of it, which is exactly what react-refresh does on every save. The + * dashboard threw on arrival, repeatably, for the whole of an editing session. + * + * So this is a STATIC guard rather than a rendering test. The failure needs a module to be + * mid-evaluation when the getter is read, and no test can arrange that from the outside: + * by the time a spec body runs, every module has finished. Babel also papers over it -- + * jest would hand back `undefined` where webpack throws. What is checkable, and what is + * actually the defect, is the cycle. + * + * Scoped to the hubs below rather than the whole tree: `src/` has dozens of long-standing + * cycles through `Common.js` and `api.js` that this PR is not untangling. Add a module here + * when it exports a `const` that others call. + */ + +const SRC = __dirname; +const EXTENSIONS = ['.js', '.jsx', '.ts', '.tsx']; + +/* + * Modules that own a subtree and export a `const` into it. Each must not be reachable from + * anything it imports. + */ +const HUBS = ['components/Calculators.js']; + +const resolve = (specifier, fromFile) => { + if (!specifier.startsWith('.')) return null; // A package, not our graph. + const base = path.resolve(path.dirname(fromFile), specifier); + const candidates = [ + base, + ...EXTENSIONS.map(extension => base + extension), + ...EXTENSIONS.map(extension => path.join(base, `index${extension}`)), + ]; + const found = candidates.find(candidate => + fs.existsSync(candidate) && fs.statSync(candidate).isFile()); + return found && EXTENSIONS.includes(path.extname(found)) ? found : null; +}; + +/* + * Static `import ... from '...'` and `export ... from '...'` only. A dynamic `import()` is + * deliberately excluded: it defers evaluation, which is one of the ways to BREAK a cycle. + */ +const IMPORT = /(?:^|\n)\s*(?:import|export)[\s\S]*?from\s+['"]([^'"]+)['"]/g; + +const importsOf = (file) => { + const source = fs.readFileSync(file, 'utf8'); + return [...source.matchAll(IMPORT)] + .map(match => resolve(match[1], file)) + .filter(Boolean); +}; + +/** The shortest import path from `start` back to itself, or null if there is none. */ +const findCycle = (start) => { + const queue = importsOf(start).map(file => [file]); + const seen = new Set([start]); + while (queue.length) { + const trail = queue.shift(); + const current = trail[trail.length - 1]; + if (current === start) return trail; + if (seen.has(current)) continue; + seen.add(current); + for (const next of importsOf(current)) { + // `start` is not in `seen`, so it is reachable again and closes the trail. + if (!seen.has(next) || next === start) queue.push([...trail, next]); + } + } + return null; +}; + +describe('import cycles', () => { + it.each(HUBS)('%s is not reachable from its own imports', (hub) => { + const start = path.join(SRC, hub); + expect(fs.existsSync(start)).toBe(true); + + const cycle = findCycle(start); + /* + * The cycle itself is the assertion's value, not a message beside it -- jest's + * `expect` takes no message argument, and the path is the only useful thing to + * read when this fails. + */ + const readable = cycle + ? `${hub} is in an import cycle:\n ${[hub, ...cycle.map(f => path.relative(SRC, f))] + .join('\n -> ')}` + : null; + + expect(readable).toBeNull(); + }); + + it('detects a cycle when there is one, and reports a real one', () => { + /* + * The walker checked against itself. Without this, the guard above passes just as + * happily when `findCycle` is broken and returns null for everything -- the failure + * mode of every static check that has nothing to find. + * + * `Common.js` is a hub with long-standing cycles this PR is not untangling, so it is + * a reliable positive. The claim is about the walker's invariants rather than which + * files it names, because naming a pair makes the test fail the day that pair is + * legitimately untangled. + */ + const start = path.join(SRC, 'components/Common.js'); + const cycle = findCycle(start); + + expect(cycle).not.toBeNull(); + // The trail closes on where it started. + expect(cycle[cycle.length - 1]).toBe(start); + // Every step is an import the walker did not invent. + [start, ...cycle].forEach((file, index) => { + if (index === cycle.length) return; + expect(importsOf(file)).toContain(cycle[index]); + }); + }); +});