Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions flip-ui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,54 @@ Client ID are required for a production deployment.
> [`vite.config.mts`](vite.config.mts)). Use `VITE_LOCAL` only with `npm run dev` against a mocked API, and never
> set `VITE_DEMO` at all — the demo build gets it from `--mode demo`. Keep both out of CI and deploy environments.

### Dependency scoping

**Production source may not import a package that is only declared in `devDependencies`, nor one that is
declared nowhere and resolves through npm's hoisting of a parent's tree.** "Production source" is everything
the deployed bundles reach: `src/`, minus its test files, plus `mocks/` — which `build:demo` pulls into the
public `/ark_demo` bundle.

This is enforced, not just documented: the `flip-ui/dependency-scoping` block in
[`eslint.config.mjs`](eslint.config.mjs) runs `import-x/no-extraneous-dependencies` over exactly that file
set, so `npm run lint` fails in CI the moment an import lands in the wrong stanza.

The rule exists for **Dependabot**, not the build. `vite build` tree-shakes from the entry points and bundles
whatever they reach whichever stanza a package sits in, and every install path in the repo (`npm ci` in CI,
the Dockerfile, `make deploy-ui`) installs both stanzas — so a wrong stanza breaks nothing and is invisible
until it matters. What it changes is the **scope label on a security alert**: a package that ships to users
but sits in `devDependencies` produces an alert labelled "Development", which reads as *not in the production
bundle* and invites a wrongly-dismissed alert on code CloudFront is serving. FLIP#1041 corrected 21
packages: 18 moved out of `devDependencies`, two that were declared nowhere at all (`codemirror`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This states "18 moved out of devDependencies, two that were declared nowhere at all (codemirror, tippy.js)". Based on the package.json/lockfile diff, vuejs-smart-table was also declared in neither stanza before this PR (no prior dependencies/devDependencies entry, no prior lockfile node) -- so it looks like the "declared nowhere" bucket should be 3, not 2, and the "moved from devDependencies" count should be adjusted down accordingly.

`tippy.js` — they resolved only by hoisting), and `husky`, which had drifted the other way. Correct scoping
is also the precondition for ever adopting `npm ci --omit=dev` here.

Note the rule keys off the **import graph, not the bundle**: a package imported by production source is a
`dependency` even if tree-shaking currently drops it. What production code imports is stable, whereas what
survives tree-shaking silently flips the correct answer whenever an unrelated component starts or stops being
used. (`@popperjs/core` was the worked example of this until FLIP#1063 removed its only importer along with
the dead `AiSelect`/`AiChipSelect` components, at which point it left the manifest entirely — the honest
resolution, and the one a lint rule cannot reach, since nothing flags an import that is merely unreachable.)

Two traps when auditing this by hand:

- **A `from "..."` grep under-reports.** `highlight.js` is loaded lazily via
`import("highlight.js/lib/core")` and `import("highlight.js/lib/languages/json")` in
[`src/utils/highlightJson.ts`](src/utils/highlightJson.ts), and is correctly a `dependency` despite having
no static import. Test files also live under `src/` (107 in `__tests__/` directories plus two flat
`*.spec.ts`), and their imports — `vitest`, `@vue/test-utils`, `@pinia/testing` — are genuinely dev-only.
- **A static import of a side-effecting module is never dropped, even from a branch that folds.** Neither
mock server may be imported statically from [`src/main.ts`](src/main.ts); both are loaded through dynamic
`import()` inside their own folded branch. `mocks/server` was static until FLIP#1041 and shipped ~230
modules — Mirage, Pretender, route-recognizer, inflected and all of lodash — in the production entry
chunk of every build, because miragejs patches `Error.prototype` at module scope. Making it dynamic cut
the entry chunk from 331 KB to 182 KB. See the comment above `bootstrap()`. This is enforced twice, because
`miragejs` is a legitimate `dependency` (the demo bundle ships it) and so invisible to the scoping rule
above: the `flip-ui/no-static-mirage` block in [`eslint.config.mjs`](eslint.config.mjs) fails `npm run
lint` on any static import of `miragejs`/`pretender`/the mock servers from `src/` (dynamic `import()`
stays legal), and [`scripts/assert-no-demo-artefacts.mjs`](scripts/assert-no-demo-artefacts.mjs) carries
Mirage/Pretender sentinels so the built artefact is checked too — the only guard that also catches a
folded branch that stops folding.

## Testing

### Unit tests (Vitest)
Expand Down
59 changes: 59 additions & 0 deletions flip-ui/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import stylistic from "@stylistic/eslint-plugin";
import { defineConfigWithVueTs, vueTsConfigs } from "@vue/eslint-config-typescript";
import importX from "eslint-plugin-import-x";
import simpleImportSort from "eslint-plugin-simple-import-sort";
import pluginVue from "eslint-plugin-vue";

Expand Down Expand Up @@ -141,4 +142,62 @@ export default defineConfigWithVueTs(
},
},
},

{
// Dependency scoping guard (FLIP#1041). Production source may not import a
// package that is only declared in `devDependencies`, nor one that is declared
// nowhere and resolves solely through npm's hoisting of a parent's tree.
//
// The point is not the build - vite bundles whatever the entry points reach,
// whichever stanza a package sits in. It is that Dependabot derives a security
// alert's scope label from the stanza, so a shipped package left in
// `devDependencies` yields an alert labelled "Development" that reads as
// not-user-facing. See flip-ui/README.md -> Dependency scoping.
//
// `mocks/` is deliberately NOT exempt: mocks/demo-server.ts ships in the public
// /ark_demo bundle, so its imports are production imports.
name: "flip-ui/dependency-scoping",
files: ["src/**/*.ts", "src/**/*.vue", "mocks/**/*.ts"],
ignores: ["**/*.spec.ts", "**/__tests__/**"],
plugins: { "import-x": importX },
rules: {
"import-x/no-extraneous-dependencies": ["error", {
devDependencies: false,
optionalDependencies: false,
peerDependencies: true,
includeTypes: false,
}],
},
},

{
// Static-import guard for Mirage (FLIP#1041 review). `miragejs` is a
// legitimate `dependency` — build:demo ships it — so the scoping rule above
// cannot flag a static re-import from production source, which is exactly the
// regression that put ~230 modules (Mirage, Pretender and all of lodash) in
// the production entry chunk. Both mock servers must stay behind a dynamic
// import() inside their folded branch (see the bootstrap() comment in
// src/main.ts); this rule ignores dynamic imports, so those stay legal.
// assert-no-demo-artefacts.mjs backstops this at the artefact level, where a
// fold failure — the class no source lint can see — would land.
name: "flip-ui/no-static-mirage",
files: ["src/**/*.ts", "src/**/*.vue"],
ignores: ["**/*.spec.ts", "**/__tests__/**"],
rules: {
"no-restricted-imports": ["error", {
patterns: [{
group: [
"miragejs",
"miragejs/*",
"pretender",
"pretender/*",
"**/mocks/server",
"**/mocks/demo-server",
],
message: "Mirage must not be statically imported from production source — load it via a "
+ "dynamic import() inside a folded branch. See bootstrap() in src/main.ts.",
}],
}],
},
},
);
4 changes: 1 addition & 3 deletions flip-ui/mocks/roles/seed-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,4 @@ export const roleResearcher: IRole = {
roledescription: "A researcher."
};

export const allRoles: IRoleResponse = {
roles: [roleAdmin, roleResearcher]
};
export const allRoles: IRoleResponse = { roles: [roleAdmin, roleResearcher] };
12 changes: 10 additions & 2 deletions flip-ui/mocks/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ const projectsModel: ModelDefinition<IProject> = Model.extend({});
const usersModel: ModelDefinition<IUser> = Model.extend({});
const detailsModel: ModelDefinition<ISiteDetails> = Model.extend({});

// eslint-disable-next-line @typescript-eslint/ban-types
// Mirage's Registry is parameterised by the model and factory maps; this app
// registers neither, so both are genuinely empty. `ban-types` was removed in
// typescript-eslint v8 and split into `no-empty-object-type` (FLIP#1041 — the
// old disable named a rule that no longer exists, so it suppressed nothing).
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
export type AppRegistry = Registry<{}, {}>;

type AppSchema = Schema<AppRegistry>
Expand Down Expand Up @@ -270,7 +274,11 @@ export const makeServer = ({ environment = "development" } = {}): Server<AppRegi
// Mirror the hub: name and organisation are withheld from this route.
const { id, email, isDisabled } = user;

return new Response(200, undefined, { id, email, isDisabled });
return new Response(200, undefined, {
id,
email,
isDisabled
});
});

// #endregion
Expand Down
Loading
Loading