Skip to content

Commit 5b9cf96

Browse files
os-zhuangclaude
andauthored
fix(plugin-map): drop the maplibre-gl@6 default import + gate type-check in CI (#2911) (#2915)
* fix(plugin-map): drop the maplibre-gl@6 default import + gate type-check in CI (#2911) maplibre-gl@6.0.0 removed its default export (#2848), so ObjectMap.tsx's `import maplibregl from 'maplibre-gl'` has been a TS1192 error on main. The binding was never used — the map comes from react-map-gl/maplibre and the CSS from the side-effect import on the next line — so it is deleted outright. Runtime-neutral: @vis.gl/react-maplibre does `Promise.resolve(mapLib || import('maplibre-gl'))`, so it self-loads the library. Browser-verified against the store-locator-map catalog schema — maplibre-gl fetched as its own lazy chunk, canvas 800x600, all 3 markers mounted, identical with and without the static import. The error survived a day of green CI because no workflow ran type-check, and `turbo build` only checks types where a package's build happens to invoke tsc (the 22 vite-build packages do not). A sweep of all 45 packages found ten with broken types. `pnpm type-check` alone is not a gate: turbo silently skips packages with no type-check script, so with plugin-map fixed it reports 63/63 green while nine packages remain broken. Therefore: - plugin-ai / plugin-report gain the tsconfig `paths` override their type-checked peers already carry (and their own vite dts program already applies), which is the sole cause of the TS6059 rootDir floods. - Seven packages gain "type-check": "tsc --noEmit"; coverage 28 -> 35 of 45. - scripts/check-type-check-coverage.mjs requires every uncovered package to be declared with a reason and only lets the lists shrink. - New "Type Check" CI job runs the coverage guard, then pnpm type-check. Both halves were proven red before being trusted: the guard in all four of its failure modes, and the job against both the re-introduced maplibre-gl import and a fresh error injected into plugin-ai — which had no type checking at all before this change. Refs #2911, #2848 Co-Authored-By: Claude <noreply@anthropic.com> * chore(type-check): link each DEBT entry to its follow-up issue Corrects two notes while wiring the links. The plugin-grid TS2367 pair is not a logic bug: `reason` is mutated only inside the `bump` closure, which TS's control-flow analysis does not track, so it stays narrowed to its initialiser. Runtime is correct and deleting the branch would silently disable the type gate. And the layout error is a half-landed feature, not a dead branch — nav type `component` is fully implemented in the renderer but absent from both `NavigationItemType` and its zod enum, so the schema rejects it and authors cannot declare one. Refs #2911, #2916, #2917, #2918, #2919 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 732b1bf commit 5b9cf96

14 files changed

Lines changed: 299 additions & 2 deletions

File tree

.changeset/type-check-gate-2911.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@object-ui/plugin-map": patch
3+
"@object-ui/plugin-ai": patch
4+
"@object-ui/plugin-report": patch
5+
"@object-ui/plugin-dashboard": patch
6+
"@object-ui/create-plugin": patch
7+
"@object-ui/console": patch
8+
---
9+
10+
fix(plugin-map): drop the `maplibre-gl@6` default import, and put type-check behind a CI gate that cannot be silently skipped (#2911)
11+
12+
`maplibre-gl@6.0.0` removed its default export (arrived via #2848, dependabot),
13+
so `ObjectMap.tsx`'s `import maplibregl from 'maplibre-gl'` has been a TS1192
14+
error on `main` for a day. The binding was never used — the map instance comes
15+
from `react-map-gl/maplibre`, and the stylesheet from the side-effect import on
16+
the next line — so the import is simply deleted rather than rewritten to
17+
`import * as`.
18+
19+
Removing it is runtime-neutral, which the issue had explicitly left unverified.
20+
`@vis.gl/react-maplibre` (what `react-map-gl/maplibre` re-exports) does
21+
`Promise.resolve(mapLib || import('maplibre-gl'))` in `components/map.js`, so it
22+
loads the library itself when no `mapLib` prop is passed. Verified in a browser
23+
against the `store-locator-map` catalog schema: `maplibre-gl` is fetched as its
24+
own lazy chunk, the WebGL canvas comes up 800x600, and all three markers mount —
25+
byte-identical probe output with and without the static import. That also matches
26+
what `apps/console/src/main.tsx` already intends, where the plugin is registered
27+
lazily specifically to keep `maplibre-gl` out of the initial bundle.
28+
29+
**The reason it survived a day of green CI is the part worth fixing.** No
30+
workflow ran `type-check` at all, and `turbo build` only checks types for
31+
packages whose `build` script happens to invoke `tsc` — the 22 `vite build`
32+
packages transpile without checking. A sweep of all 45 packages found ten with
33+
broken types, `plugin-map` merely being the one that had a script to notice it.
34+
35+
Adding a `pnpm type-check` job alone would not have been a gate: **turbo silently
36+
skips any package with no `type-check` script**, so 17 packages read as passing
37+
because nothing ran. With `plugin-map` fixed, `pnpm type-check` reports 63/63
38+
green while nine packages are still broken. So:
39+
40+
- `plugin-ai` and `plugin-report` gain the `paths` override their type-checked
41+
peers already carry, which detaches workspace deps from sibling *source* and
42+
resolves them through built `.d.ts` — the sole cause of the 104-error TS6059
43+
`rootDir` floods, and the same trick their own `vite.config.ts` already applies
44+
to the dts program.
45+
- Seven packages gain `"type-check": "tsc --noEmit"` (`plugin-ai`,
46+
`plugin-report`, `plugin-dashboard`, `create-plugin`, `console`, and the two
47+
console examples). Coverage goes 28 -> 35 of 45.
48+
- New `scripts/check-type-check-coverage.mjs` makes the invisibility impossible:
49+
a package with no `type-check` script must be declared, with a reason, and the
50+
lists only shrink — gaining a script without deleting the entry fails the
51+
guard. The nine known-broken packages are recorded there with error counts
52+
(`@object-ui/runner` has no `tsconfig.json` at all), tracked as follow-ups.
53+
- New `Type Check` CI job runs the coverage guard first (instant, no install),
54+
then `pnpm type-check`.
55+
56+
Both halves were proven to fail before being trusted: the guard was exercised in
57+
all four of its failure modes, and re-introducing the `maplibre-gl` import turns
58+
the job red again, as does a fresh error injected into `plugin-ai` — a package
59+
that had no type checking whatsoever before this change.

.github/workflows/ci.yml

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,60 @@ jobs:
3535
- name: Verify all packages are in changeset fixed group
3636
run: node scripts/check-changeset-fixed.mjs
3737

38+
# Types were entirely unguarded until #2911: no job ran `type-check`, and
39+
# `turbo build` only checks types for packages whose build script happens to
40+
# invoke `tsc` — the 22 `vite build` packages transpile without checking. A
41+
# `maplibre-gl@6` breakage therefore sat on `main` for a day behind a green CI.
42+
#
43+
# Note that `pnpm type-check` alone is not a sufficient gate: turbo silently
44+
# skips any package with no `type-check` script, so a package without one
45+
# reads as passing. The coverage guard is what makes that impossible, and it
46+
# runs first because it is instant and catches the structural mistake.
47+
type-check:
48+
name: Type Check
49+
runs-on: ubuntu-latest
50+
timeout-minutes: 20
51+
52+
steps:
53+
- name: Checkout code
54+
uses: actions/checkout@v7
55+
with:
56+
submodules: true
57+
58+
- name: Enable Corepack
59+
run: corepack enable
60+
61+
- name: Verify pnpm version
62+
run: pnpm --version
63+
64+
- name: Setup Node.js
65+
uses: actions/setup-node@v7
66+
with:
67+
node-version: '20.x'
68+
cache: 'pnpm'
69+
70+
# Every package must be either type-checked or explicitly declared as a
71+
# known gap. Runs before install: it only reads package.json files.
72+
- name: Verify type-check coverage
73+
run: node scripts/check-type-check-coverage.mjs
74+
75+
- name: Install dependencies
76+
run: pnpm install --frozen-lockfile
77+
78+
- name: Turbo Cache
79+
uses: actions/cache@v6
80+
with:
81+
path: node_modules/.cache/turbo
82+
key: turbo-${{ runner.os }}-type-check-${{ github.sha }}
83+
restore-keys: |
84+
turbo-${{ runner.os }}-type-check-
85+
turbo-${{ runner.os }}-
86+
87+
# `type-check` dependsOn `^build`, so this builds workspace dependencies
88+
# first: the checked packages resolve their deps through built `.d.ts`.
89+
- name: Run type-check
90+
run: pnpm type-check
91+
3892
# PRs run the suite split across 4 runners. The suite is dominated by fixed
3993
# per-file cost rather than by the assertions: a PR run reported 495s wall
4094
# clock of which only 178s was `tests` — the rest was `setup` (831s

apps/console/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"dev": "vite",
4646
"build:plugin": "tsc -p tsconfig.plugin.json",
4747
"build": "tsc && vite build && pnpm build:plugin",
48+
"type-check": "tsc --noEmit",
4849
"build:analyze": "pnpm build && echo 'Bundle analysis available at dist/stats.html'",
4950
"preview": "vite preview",
5051
"test": "vitest run",

examples/byo-backend-console/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"scripts": {
88
"dev": "vite",
99
"build": "tsc && vite build",
10+
"type-check": "tsc --noEmit",
1011
"preview": "vite preview"
1112
},
1213
"dependencies": {

examples/console-starter/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"scripts": {
88
"dev": "vite",
99
"build": "tsc && vite build",
10+
"type-check": "tsc --noEmit",
1011
"preview": "vite preview"
1112
},
1213
"dependencies": {

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"test:coverage": "vitest run --coverage",
3232
"lint": "turbo run lint",
3333
"type-check": "turbo run type-check",
34+
"type-check:coverage": "node scripts/check-type-check-coverage.mjs",
3435
"cli": "node packages/cli/dist/cli.js",
3536
"objectui": "node packages/cli/dist/cli.js",
3637
"create-plugin": "node packages/create-plugin/dist/index.js",

packages/create-plugin/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
],
1717
"scripts": {
1818
"build": "tsup",
19+
"type-check": "tsc --noEmit",
1920
"dev": "tsup --watch",
2021
"test": "vitest run",
2122
"lint": "eslint ."

packages/plugin-ai/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
],
2121
"scripts": {
2222
"build": "vite build",
23+
"type-check": "tsc --noEmit",
2324
"clean": "rm -rf dist",
2425
"test": "vitest run --passWithNoTests",
2526
"lint": "eslint ."

packages/plugin-ai/tsconfig.json

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
"extends": "../../tsconfig.json",
33
"compilerOptions": {
44
"outDir": "dist",
5-
"jsx": "react-jsx"
5+
"jsx": "react-jsx",
6+
"baseUrl": ".",
7+
// Overriding `paths` drops the root config's `@object-ui/* -> packages/*/src`
8+
// mappings, so workspace deps resolve through their built `.d.ts` instead of
9+
// pulling sibling sources into this program (which would be outside rootDir).
10+
// vite.config.ts already does the same for the dts program.
11+
"paths": {
12+
"@/*": ["src/*"]
13+
},
14+
"skipLibCheck": true
615
},
716
"include": ["src"],
817
"exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"]

packages/plugin-dashboard/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
},
1717
"scripts": {
1818
"build": "vite build",
19+
"type-check": "tsc --noEmit",
1920
"test": "vitest run",
2021
"lint": "eslint ."
2122
},

0 commit comments

Comments
 (0)