Uh oh!
There was an error while loading. Please reload this page.
refactor(app-shell): delete the dead metadataConverters module - #6265
Merged
yinlianghui merged 2 commits intoAug 25, 2026
Conversation
`packages/app-shell/src/utils/metadataConverters.ts` has zero importers, is not re-exported from the package barrel, and no `exports` subpath reaches it. Enforce-or-remove: it is not merely unused, it is a copy-paste source for two defects this lane just repaired --- a name-heuristic `isSystem` and a three-way `referenceTo` tolerance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
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
|
yinlianghui
marked this pull request as ready for review
August 25, 2026 06:24
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui
deleted the
claude/issue-6224-delete-dead-metadataconverters
branch
August 25, 2026 06:35
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
2 participants
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#6224
Deletes
packages/app-shell/src/utils/metadataConverters.ts(139 lines). No other production change.Why this is a delete, not a shrug
The module is not merely unused — it is a copy-paste source for two defects this lane repaired hours ago, and it is exactly the file a future author looking for "the converter" would find and revive:
isSystem. Today's file computesisSystem: field.readonly === true && (field.name === 'id' || field.name === 'createdAt' || field.name === 'updatedAt')— a guess from the field's name, on top of a spelling (
createdAt) the spec does not use (created_at). PR fix(designer,app-shell): spell the designer's lookup target and system marker as the spec does (#6041, #6044) #6225 (Both designer field shapes spell the spec'ssystemflag asisSystem— so system-field detection reads a key the server never sends, and the key round-trips back out as a 422 #6044) just landed the repair that reads the server's realsystemflag instead, inMetadataFieldsPage.toDesignerField.referenceTotolerance. The module'sMetadataFieldinterface declaresreferenceTo, and the converter readsfield.reference_to || field.reference || field.referenceTo— a three-way tolerance for a target the spec spellsreference. PR fix(designer,app-shell): spell the designer's lookup target and system marker as the spec does (#6041, #6044) #6225 (Field designer offers areferenceTocontrol for lookup fields — a key FieldSchema rejects by name, so saving a lookup field hard-blocks the object #6041) retired that misspelling and addedreferenceTotoRETIRED_FIELD_KEYSso a stored one cannot ride back out.Both patterns were re-read in the file at
9b61cf199before writing this, so the argument is about today's code, not the code the finding was filed against. Enforce-or-remove: dead code carrying wrong patterns is worse than dead code.Verification — and why the green suite is not the evidence
1. Zero importers, measured with two working positive controls
A grep that finds nothing is indistinguishable from a grep that is wrong, so the instrument was proven first.
*.ts/*.tsx, excludingnode_modules/dist)services/MetadataService.ts(control)utils/appRoute.ts(control, same directory)utils/resolveViewId.ts(control, same directory)utils/crudAffordances.ts(control, same directory)utils/metadataConverters.tsWidening the sweep past static imports, as the card asked:
metadataConvertersacross all file types returns only prose —CHANGELOG.mdentries,ROADMAP.mdlines 1000/1006. Zero source hits.require(...), no dynamicimport(...), and no template-literal import path mentions the module anywhere in the repo.packages/app-shell/srccontains norequire(at all.toObjectDefinitionhits inpackages/plugin-designer/src/MetadataObjectsPage.tsxare its own localfunction toObjectDefinition(raw: ServerObjectSchema)at line 85, not an import of this module — an independent same-named function, which is itself the duplication this card is about.2. No published re-export — which is what makes this a
patchpackages/app-shell/src/index.ts: no mention ofmetadataConverters,toObjectDefinition, ortoFieldDefinition.packages/app-shell/src/utils/index.ts(the barrel the package root re-exports from): same, zero hits across all 112 lines.packages/app-shell/package.jsonexportsdeclares exactly two subpaths:"."and"./styles.css".dist/index.d.ts(not a stale one): zero hits for all three symbols, while a control grep for two symbols that are published (resolveHostAppSegment,MetadataDirectoryPage) hits.Worth naming: the module did compile into
dist/utils/metadataConverters.{js,d.ts}and therefore shipped in the tarball (files: ["dist"]). It was still unreachable — theexportsmap refuses every subpath except the two above, so no external consumer could deep-import it. Removing shipped-but-unreachable bytes is not a breaking change, which is why this is apatchand not theminor-with-a-loud-changeset shape #6011 had.3. The type-check oracle, with the closure built first
tscresolves workspace deps throughdistwhilevitest.config.mtsaliases@object-ui/*tosrc/, so a red here can be an unbuilt sibling rather than a real consumer. The closure was built first, both runs were--force(a cached turbo green is not a measurement), and the staledist/utils/metadataConverters.{js,d.ts}were deleted before the post-deletion run — otherwise a consumer resolving throughdistwould have type-checked green against a file that no longer exists insrc.turbo run type-check --filter='...@object-ui/app-shell' --force)Tasks: 39 successful, 39 total·Cached: 0 cacheddistremoved, same commandTasks: 39 successful, 39 total·Cached: 0 cached· zeroerror TS…linesScope of that filter is the downstream direction (prefix
...pkg):@object-ui/app-shellitself plus its three consumers —@object-ui/console,@object-ui/example-byo-backend-console,@object-ui/example-console-starter.No hidden consumer surfaced, so the card's fork clause was not triggered.
4. Proving the oracle can fail
A green type-check only means something if it could have gone red. A probe file importing the deleted specifier was added to
packages/app-shell/src/utils/, its landing on disk confirmed by grepping for both the injected marker and the import specifier (an editor's exit code is not evidence a write landed), thenpnpm --filter @object-ui/app-shell type-check:Exactly one error, on the probe. The probe was restored under
trap … EXIT INT TERMusing absolute paths, and the restore was verified, not assumed —git diff HEAD --statafterwards showed only the intended deletion (1 file changed, 139 deletions(-)).5. Tests
pnpm exec vitest run packages/app-shell/src/utils --maxWorkers=2, run from the repo root (objectui#3378: the--filter/cd-into-package forms either refuse loudly or silently runapps/console's 22 files and report them green):Sanity-checked against the target's own count:
find packages/app-shell/src/utils -name '*.test.ts*'= 26, matching — so this is app-shell'sutils, not another package's suite.Declared narrowing: app-shell's full suite is 524 test files; this run covers the 26 under the deleted file's own directory. CI's 4-shard
testjob runs the rest either way, and the blast radius is measurably zero — no file in the repo imports the module by any static or dynamic route, so no test can transitively load it.6. Gates
Derived by enumerating each CI job's own step list (
ci.yml,lint.yml,changeset-presence.yml,control-bytes.yml,readme-exports.yml,node-esm-load-gate.yml) against the two files this PR actually changes. Verdict lines below are the gates' own, re-run at final HEAD4473c3149:check-changeset-presence— measured red → green, the one real red-to-green in this PR. Before: "❌ 1 source file(s) of 1 released package(s) changed, and this change adds no changeset:@object-ui/app-shell". After: "✅ 1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)". The gate decided the changeset is owed; thepatchscoring follows from §2.check-changeset-no-major— "✅ No changeset declares amajorbump."check-changeset-fixed— "✅ All workspace packages are in the changeset fixed group."check-control-bytes— "✅ OK (scanned 5168 tracked text file(s); skipped 85 binary)."check-readme-exports— "✅ OK … 378 self-imports judged (378 real, 0 wrong-path, 0 fabricated) … 0 unbuilt". This one first came back red with 5 unjudgeable self-imports in@object-ui/cliand@object-ui/plugin-ai— "its type entry./dist/index.d.tsis not on disk". That is the unbuilt-sibling artefact, not this change; rather than assume so, those two packages were built and the gate re-measured green.app-shell's README never mentioned the removed symbols.check-designer-field-key-parity— "designer-field-key-parity: OK" (run because the module models designer field defs).check-phantom-dependencies— "✅ Every in-scope import is declared by the package that publishes it." (the deleted file's only import was@object-ui/types, which app-shell still uses widely).check-package-self-import— "✅ No package names itself inside its own src/."check-spec-symbol-derivation— "✅ spec alignment claims: 2 declared deliberate copies, 18 unbacked claims in 5 packages."check-node-esm-load --specifiers-only— "Specifier leg: no un-ledgered package emits an extensionless relative specifier."check-lint-coverage— "✅ 46/46 packages linted, 0 with outstanding errors."check-type-check-coverage— "✅ 41/41 packages compile their tests, 0 declared debt."pnpm --filter @object-ui/app-shell lint(eslint .) — "✖ 2678 problems (0 errors, 2678 warnings)"; zero errors, warnings are the package's pre-existingno-explicit-anybaseline.turbo run build --filter='!@object-ui/site') — "43 successful, 43 total".What would still pass on a revert
Stated plainly, because it is the honest weakness of a deletion card: every assertion in this repo would still pass if this commit were reverted. Nothing tests the deleted module, which is precisely the finding. The claim this PR makes is not "a test now passes" but "nothing reaches this code" — and that claim rests on the positive-control greps (§1), the published-surface check against a freshly built
dist(§2), the downstream type-check with a built closure and staledistcleared (§3), and the oracle control proving that type-check would have gone red on a real consumer (§4). The one genuine red-to-green here ischeck-changeset-presence(§6).Out of scope
The issue's "adjacent, same family" note —
packages/plugin-dashboard/src/recordFields.tsx:56readingdef.isSystem === true || def.system === true, where theisSystemhalf can never fire on a spec-parsed field def — is not addressed here; it is live code in another package and stays open on #6224's thread for triage.packages/plugin-designer/src/MetadataObjectsPage.tsx's independent localtoObjectDefinitionis likewise untouched.Collision check
packages/app-shell/src/utils/is this card's lane. #6165'sHomeRailconsumers do live inapp-shell, but underpackages/app-shell/src/console/home/(HomeRail.tsx,HomePage.tsx, plus tests) — different files, no file-level overlap with the single file this PR removes. No overlap with #6176 (plugin-form), #6167 (examples/schema-catalog,apps/site), or #5935 (components,plugin-list,plugin-detail, the lucide gate script).Generated by Claude Code