fix(grid): let the object-schema policy outrank row-key column derivation - #6722
Merged
os-sales merged 1 commit intoAug 29, 2026
Merged
Conversation
…tion `ObjectGrid.generateColumns()` checked the inline-data path before the object-schema path. That path is gated on `hasInlineData` (`dataConfig.provider === 'value'`), and `dataConfig` is built as `provider: 'value'` from the `data` prop before anything else, so it is taken by every grid whose rows were handed down rather than fetched. It returned unconditionally whenever rows were present, and its projection is `schemaFields || Object.keys(inlineData[0])` — the first row's keys. The object-schema path, which carries the documented default-columns policy, was therefore unreachable for every object-bound grid reached through a fetching host. Measured on one page/source/object with one variable: grid-fetches rendered 5 policy columns, host-fetches rendered 10, adding `id` (`hidden: true`) and the four `system` audit columns. Only the row-key fallback yields, and only to a loaded policy (`!schemaFields && !!objectName && !!objectSchema`). An authored `fields` projection still wins, because the schema path drops names the object does not declare and a host may join or derive keys. Requiring the *loaded* schema avoids a worse defect: `objectSchema` is null until an async fetch lands, so gating on `objectName` alone would paint an empty header row on first paint and then flip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 29, 2026 01:35
os-sales
deleted the
claude/issue-6677-grid-default-columns-branch-order
branch
August 29, 2026 03:00
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 free
to 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 #6677
ObjectGrid.generateColumns()checks three default paths in order: authoredcolumns→ the inline-data path → the object-schema path. The inline-data pathis gated on
hasInlineData(dataConfig.provider === 'value'), anddataConfigis built as
provider: 'value'from thedataprop before anything else —so it is taken by every grid whose rows were handed down rather than fetched. It
returned unconditionally whenever rows were present, and its projection is
schemaFields || Object.keys(inlineData[0]), the first row's keys.That made the object-schema path — the one carrying the documented policy
(
highlightFieldsfirst; else drophidden, drop readonly system-managed, pushthe remaining system/ownership columns to the end) — unreachable for every
object-bound grid reached through a fetching host (
ListView,ObjectView,…). The branch that knows the object was the one that never ran.
Re-derived from code on
origin/main@98188c284rather than from the card'sline references, and reproduced as a test before any source edit.
The change
One gate, in
packages/plugin-grid/src/ObjectGrid.tsx:The legacy path is reordered, not deleted, and the yield is as narrow as the
defect: only the row-key fallback is wrong for an object-bound grid, so only
that is given up, and only once there is a policy to give it up to. Plus the
comment block explaining why, and
objectNamenamed explicitly in the hook'sdependency array (it is derived from
dataConfig+schema.objectName, bothalready listed, so this adds no invalidation).
Why not gate on
objectNamealone — the transition the order asked aboutobjectSchemaisuseState(null)filled by an async fetch, so it isnullonfirst paint even when
objectNameis set. Gating the legacy path onobjectNamealone falls straight through to
if (!objectSchema) return []and paints anempty header row, then flips to five columns — a worse defect than the one
being fixed. Requiring the loaded schema keeps the row-key columns on screen
until the object is actually known.
Measured, not reasoned: with
getObjectSchemaheld on a pending promise thegrid renders the ten humanised row-key columns and zero is never observed; when
the promise resolves it becomes the five policy columns. Both directions are
pinned. The same condition is the graceful fallback when the schema fetch fails
or the data source has no
getObjectSchema— the existingcatchalreadypromises "columns will still fall back to heuristic inference", and now that
stays true instead of the grid going blank.
Why an authored
fieldsprojection still winsThe schema path drops a name the object does not declare (
if (!field) return;),and a host may legitimately join or derive keys.
!schemaFieldsis exactly thecondition under which the
||reaches for the row keys, so the gate cannot driftfrom the fallback it guards. Pinned with a
fields: ['name', 'computed_score']case where
computed_scoreis not an object field.Anti-vacuity
packages/plugin-grid/src/__tests__/hostFetchedDefaultColumns-6677.test.tsxreproduces the card's measurement: the
opportunityobject withidmarkedhidden: trueand the four audit fieldssystem+readonly, rows handed inthrough the
dataprop exactly as a fetching host passes them.RED-FIRST on the merge-base
98188c284, this file only — 3 red / 4 green:The extras are appended to a superset, not substituted for the five, so the
test asserts the absence of
Id/Created At/Created By/Updated At/
Updated Byas well as the presence of the right five — a presence-only checkpasses on the broken build. The four greens are boundaries the reorder must not
cross and are green in both worlds: inline data with no object behind it, an
authored projection naming an audit column on purpose, an authored projection
naming a non-schema key, and the schema-in-flight first paint.
After the fix: 7/7 green in that file.
⛔
(col as any).fitContentin the grouped-width pass is untouched, andcolumnReadBoundary-6458.test.tsis untouched — its control is not re-pointedinto the region this PR changes.
packages/plugin-listis untouched.Verification
Union re-run on this PR's final commit
efbd027cb:pnpm exec vitest run packages/plugin-grid/pnpm exec vitest run packages/app-shell/pnpm --filter @object-ui/plugin-grid run type-checktsc --noEmit && tsc -p tsconfig.test.json)npx eslint .inpackages/plugin-gridpnpm check:control-bytespnpm check:vi-mock-specifiersnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsnode scripts/check-lint-coverage.mjsDownstream consumers
plugin-list+plugin-view+plugin-designerran green(92 files, 1069 passed) on a tree byte-identical to
efbd027cbunderpackages/**— only the.changeset/*.mdwas added after that run.The
type-checkclaim is measured, not assumed:tsc -p tsconfig.test.json --listFileslists both edited files (ObjectGrid.tsxand the new test), so thegreen covers them rather than excluding the test file.
Declared narrowing — lint
Repo-wide
pnpm lint(turbo run lint) was not run locally; CI runs thefarm regardless. The narrowing is to
packages/plugin-grid, with the evidencethat makes it a measurement rather than a gap:
count —
eslint .inside the package resolves the root flat config'signoresandfiles: ['**/*.{ts,tsx}'].--format json: 135 files inspected, 0 errors.eslint.config.jssetslanguageOptionstoecmaVersion+globalsonly — there is noparserOptions.project/projectService, so type-aware linting is notenabled and no rule reads across file boundaries. This diff therefore cannot
move the verdict of any file it does not touch, and
packages/plugin-gridisthe only package whose
.ts/.tsxinputs changed.react-hooks/exhaustive-depsstill warns at the edited hook (line 2423) aboutnavigationandschema.showColumnTypeIcons— pre-existing, and adding adependency cannot create a missing-dependency report for other identifiers.
ESLint fails only on errors here (no
--max-warnings, deliberately), and errorsare 0.
Changeset
patchon@object-ui/plugin-grid, scored deliberately. No public API moves —no prop, type, export or signature change — and this restores the
default-columns policy the component already documents and already applied
whenever the grid fetched its own rows; the host-fed divergence was the defect,
not a contract.
minorwas considered, because the visible column set changes onexisting screens, and rejected: the lost columns were never declared by any
author, only leaked by the branch order, and this repo scores behaviour-
correcting fixes as patch and reserves
minorfor new capability. Amajorisnever authored here — the fixed group tracks
@objectstack.Generated by Claude Code