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]); + }); + }); +});