Uh oh!
There was an error while loading. Please reload this page.
refactor(core,react,dashboard,report): the analytics label net's fetch-and-memo glue is written once (#4389) - #4404
Merged
Conversation
…h-and-memo glue is written once (#4389) PR #4388 (objectui#4330) left the same React glue on two surfaces: the dashboard's `DatasetWidget` and plugin-report's `useDatasetDimensionLabels`. The resolution RULES were never duplicated — both call the same `@object-ui/core` helpers — but the wiring around them was: read the object schema through the host's authenticated `apiFetch`, keep the fetched metadata locale-free in state, derive the label maps in a render memo. Two copies meant two statements of the same two bug fixes, which is a drift surface rather than a defect. Filed as objectui#4389 and retired here. Split along the layer that can hold each half: `@object-ui/core` (React-free) loadDimensionFieldMeta base-object read composed with the dimension walk deriveDimensionLabelMaps the locale-APPLYING derivation dimensionOptionTranslator binds the bundle resolver to the object that OWNS the terminal field (the relationship TARGET for a dotted path, which is the key the bundle uses) `@object-ui/react` (the React wiring core structurally cannot hold) useDatasetDimensionMeta the locale-free read: context, state, effect useDatasetDimensionLabels the above + the derivation, behind one memo `DatasetWidget` consumes `useDatasetDimensionMeta` and layers its CHART-ONLY per-category colour and category-order derivation on top — a table renders no palette, so that stays local and `isTable`-gated exactly as before. plugin-report's file becomes a re-export, keeping its rationale header and its import path so `DatasetReportRenderer`'s three call sites are untouched. The card named `@object-ui/core` as the whole glue's home. That home was disproven by measurement and retired in the card's PM RULING #2: `SchemaRendererContext` is defined at `packages/react/src/context/SchemaRendererContext.tsx:19` and exported by `@object-ui/react`, which declares `@object-ui/core` in `dependencies`, so core importing it back is a cycle; core is React-free by declaration (no `react` dep or peerDep), by content (0 `.tsx`, 0 react imports, 0 hooks) and by AGENTS.md §3. objectui#3367 had already ruled this direction for the same family. Note that `react` DOES resolve from `packages/core` via the root devDependency symlink — a resolution-based direction check returns the wrong answer here, filed separately as objectui#4394. Behaviour is unchanged by construction: same read count, same best-effort fallback (a failed read leaves the server's rows), same memo boundary. The two bug fixes are now singly expressed and pinned at the shared hook: - the read rides the host's AUTHENTICATED `apiFetch` (objectui#4121) — pinned by asserting a NEW channel re-issues the read, i.e. that `apiFetch` really is in the effect's deps, which "was apiFetch called" cannot see; - the fetched metadata stays LOCALE-FREE (objectui#4030 / PR #4324) — pinned by switching language at runtime and asserting the labels flip in place with no second metadata read. All 39 assertions PR #4388 landed across both surfaces pass UNCHANGED, and their files are byte-identical to origin/main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 12, 2026
CollaboratorAuthor
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 03:09
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 12, 2026
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4389
PR #4388 (#4330) left the same React glue on two surfaces. The resolution rules were never duplicated — both call the same
@object-ui/corehelpers — but the wiring around them was: read the object schema through the host's authenticatedapiFetch, keep the fetched metadata locale-free in state, derive the label maps in a render memo. Two copies meant two statements of the same two bug fixes. Nothing a user hits today; retired deliberately, per the card.Hook home, and the dependency-direction evidence
The card's title said the home is
@object-ui/core. That is disproven, and the card's PM RULING #2 retires it. Measured onorigin/main, two independent blockers:1. Hard cycle.
SchemaRendererContextis defined atpackages/react/src/context/SchemaRendererContext.tsx:19, exported viacontext/index.tsthensrc/index.ts.packages/react/package.jsondeclares"@object-ui/core": "workspace:*"independencies. So core importing it back is a cycle, not a new edge.useSafeFieldLabelis the same story: it lives inpackages/i18n/src/useObjectLabel.ts:601, and@object-ui/i18nis not a core dependency either.2. Layer violation.
@object-ui/coreis React-free by declaration and by content — dependencies are only@object-ui/types,@objectstack/formula,@objectstack/spec,lodash,zod;peerDependenciesis empty;packages/core/srchas 0 files with a.tsxextension, 0reactimports and 0 exported hooks;packages/core/node_moduleshas no react. AGENTS.md section 3 pins it: "No UI-lib deps. Logic only." #3367 already ruled this exact direction for this exact family (core-canonical logic, react re-exports — never core importing react).reactDOES resolve frompackages/core, via the rootdevDependencies.reactsymlink. A resolution-based direction check therefore returns the wrong answer here; the authoritative check is the declared dependencies. Filed separately as #4394.So the split is A+B, as ruled — each half in the layer that can actually hold it:
@object-ui/coreloadDimensionFieldMeta,deriveDimensionLabelMaps,dimensionOptionTranslator@object-ui/reactuseDatasetDimensionMeta,useDatasetDimensionLabels@object-ui/reactis the one package downstream of core (helpers reachable), downstream of@object-ui/i18n(useSafeFieldLabelreachable), owner ofSchemaRendererContext(apiFetchis a local read), and upstream of both plugins. It already hosts this exact fetch-and-memo-off-apiFetchshape three times:useViewData,useElementDataSource,useDiscovery.Consumers
DatasetWidgetconsumesuseDatasetDimensionMetaand layers its chart-only colour/order derivation locally — a table renders no palette, sofirstDimPathstaysisTable-gated exactly as before.useDatasetDimensionLabels.tsbecomes a literal one-line re-export, keeping its rationale header and its import path, soDatasetReportRenderer's three call sites are untouched.Two exports rather than one is what lets that re-export stay literal:
useDatasetDimensionLabelsis the labels-only signature plugin-report already had, anduseDatasetDimensionMetais the same glue stopping one step earlier, for the surface that derives more from the metadata. The fetch-and-memo wiring itself exists once.The two properties, now singly expressed and pinned
Both were previously written out per surface — the drift this card names:
apiFetch(DatasetWidget's option-color / dimension-label probe reads the globalfetch, bypassing the host'sapiFetchchannel (the plugin-dashboard twin of #4114) #4121) — pinned by asserting a new channel re-issues the read, i.e. thatapiFetchreally is in the effect's deps. "WasapiFetchcalled" cannot see that half.New pins live at the shared seam (
packages/react/src/hooks/__tests__/useDatasetDimensionLabels.test.tsx,packages/core/src/utils/__tests__/chart-series.labelNetGlue.test.ts) because a plugin-level pin can only observe its own copy — which is how the duplication survived long enough to be filed.Pins-unchanged proof (the acceptance evidence)
All 39 assertions PR #4388 landed pass unchanged, and their files are byte-identical to
main:packages/core/ packages/react/ packages/plugin-dashboard/ packages/plugin-report/turbo run type-check --concurrency=2error TSlinton the 4 packagesno-explicit-anywarnings)check-control-bytesTypecheck ran both tsc projects per package (
tsc --noEmit && tsc -p tsconfig.test.json) wherever a test project exists. The repo-wide pass covers every package, so it subsumes a directional consumer sweep in both directions.Reverse verification
RV1 — revert
DatasetWidgetto its local glue (git checkout origin/main -- DatasetWidget.tsx), run the dashboard pins. Predicted GREEN before running, and measured GREEN: 5 files, 32 passed. Reporting the null result honestly rather than dressing it as a catch — this refactor is behaviour-preserving by construction, so the surface pins cannot distinguish shared glue from duplicated glue. That is precisely why the new hook-level pins had to be added; the #4388 pins are evidence that nothing broke, never evidence that the glue is shared.RV2 — break the locale-free property in a scratch commit (put the locale-derived
fieldOptionLabelinto the fetch effect's deps, the pre-#4324 defect). Predicted RED on the read-count assertion; measured RED, and harder than predicted: all 4 hook pins go red, because that value's identity is unstable across renders, so the effect does not refetch once per language switch — it refetches unboundedly. Observed287,436and2extra reads where 1 was expected. Restored; pins green again.Scope
Surface:
packages/core,packages/react(the new hook plus its barrel line, nothing else),packages/plugin-dashboard,packages/plugin-report, changeset. Changeset ispatchacross the 4 packages, nevermajor. Mergedorigin/main(#4383 landed mid-task, different surface) with no conflicts, and re-ran the full suite after the merge — same 2670 passed.Generated by Claude Code