fix(app-shell): re-point saveAdvisoryToast's TranslateFn at its one authority - #8205
Merged
Merged
Conversation
…uthority `TranslateFn` had three declarations, and `KNOWN_COLLISIONS` in `scripts/__tests__/one-authority-per-exported-name-6273.test.ts` carried all three as accepted debt. That baseline is SHRINK-ONLY by its own text, and nothing owned shrinking it. Re-measured first, because the card's own caution is the one that matters: a re-export of a type that had quietly diverged would silently change two call sites' contracts, and the gate counts declarations, not shapes. All three declarations are byte-identical on this base — 86 bytes each, one sha256 (29f2f334f47b8ac55bd05fc5dcf7f0d769c303f0e2deb1c79aff6a9cd46e3762) across the three, identical `od -c` dumps. So there is no shape to reconcile. The authority did not need choosing either: `AdapterProvider` is the single caller of all three emitters and already imports `TranslateFn` from `./writeWarningToast.js`, passing that one value into each of them. `saveAdvisoryToast.ts` now re-exports it, which the gate explicitly does not count as a declaration, and the baseline entry loses that site in the same change. The `packages/fields/src/widgets/file-size-guard.ts` site is deliberately NOT re-pointed and stays on the baseline. `@object-ui/app-shell` DEPENDS ON `@object-ui/fields`, so a re-export in that direction is a package cycle, and `TranslateFn` is on neither package's published face, so there is nothing to import either. Retiring the last copy means moving the authority DOWN into a package both depend on, which publishes a new name and is a decision nobody has made. Type-level only, on a name no package publishes, so the changeset declares an empty frontmatter: no package is released by this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
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
|
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
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.
Part of #8165 — deliberately not a closing keyword: this lands two of the three sites and leaves the third open, for the reason measured below. #8165 stays open for re-triage.
What changed
packages/app-shell/src/providers/saveAdvisoryToast.ts— the localexport type TranslateFn = …is gone; the module nowimport types it from./writeWarningToast.jsfor its own two uses and re-exports it, so an import from./saveAdvisoryToast.jsstill resolves to the same type. Same shape as the two worked examples already in the tree (orgRoleLabel.tsre-exportingOrgTranslate,resolveBulkActions.tsre-exportingNamedActionDef) and asmetadataReadWarningToast.ts, which took this exact route in fix(data-objectstack): discriminate a refused mapping read from a served zero, on a channel #8152.scripts/__tests__/one-authority-per-exported-name-6273.test.ts— theTranslateFnKNOWN_COLLISIONSentry loses that site. Three files to two. ⛔ No line added; the baseline is SHRINK-ONLY by its own text..changeset/8165-translatefn-one-authority.md— empty frontmatter (declares no release), see below.The card's own caution is the load-bearing one: the gate counts declarations, not shapes, so a re-export of a type that had quietly diverged would silently change two call sites' contracts with nothing noticing. Re-measured on
158d75bb3before touching anything — the three declarations are byte-identical:No shape to reconcile, so the re-point is safe on the two sites it touches.
The authority did not need choosing either:
AdapterProvideris the single caller of all three emitters and already importedTranslateFnfrom./writeWarningToast.js, passing that one value into each of them (:17,:81,:94,:107).⛔ Why the third site is NOT re-pointed — the card's remedy is dependency-illegal there
The card prescribed
export type { TranslateFn } from '<writeWarningToast>'for both of the other two. That is not available inpackages/fields:@object-ui/app-shellDEPENDS ON@object-ui/fields(packages/app-shell/package.json,dependencies), andpackages/fieldshas zero real imports of@object-ui/app-shell. Pointing fields at app-shell is a package cycle.TranslateFnis on neither package's published face, so there is nothing to import either — see the next section.⇒ retiring that copy means moving the authority DOWN into a package both depend on — the route the baseline's own
KanbanSchemanote records. That publishes a new name from a shared package and is a decision nobody has made, so this PR stops rather than guessing. Measured: all the packages carrying a copy of this shape depend on@object-ui/i18n; not all depend on@object-ui/types.Clause ② — is any of the three on a published face?
No, so this is not a public-face change and does not want
needs:contract-review. Checked for star re-exports, not only the literal name:packages/app-shell/src/index.ts— 83 export statements, zeroexport *anywhere in the file, and no literalTranslateFn. The package's only entry is.(plus./styles.css).packages/fieldshas nosrc/index.ts— its entry issrc/index.tsx, which does star 50 widget modules, but not./widgets/file-size-guard.js, and none of that file's three importers (FileField,ImageField,LocationField) re-exports the name.packages/fields/dist/index.d.tshas 0 occurrences ofTranslateFn, against a control (FileField, a name known to be starred) at 2.The gate's own verdict lines, before and after
Ablation — the gate still fails, in both directions
Both legs mutated on disk (proved by anchored fixed-string counts before and after), restored with
git checkout HEAD -- <abs path>, and the restore proved by state (git hash-object==git rev-parse HEAD:PATH, emptygit diff HEAD) rather than by an exit code. Run ataa1114558.Leg A — re-add a local declaration in
saveAdvisoryToast.ts(decl count 0 to 1, re-export 1 to 0). Gate exit 1:Leg B — put the three-site baseline entry back while keeping the re-point (entry3 count 0 to 1, entry2 1 to 0). Gate exit 1, which is the proof the baseline edit was owed, not optional:
Restore, both legs and final state:
Tests and gates run locally (all at
aa1114558unless noted)pnpm exec turbo run build --filter=@object-ui/app-shell^... --concurrency=2pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json)VERDICT command-exit 0, zeroerror TSpnpm exec vitest run packages/app-shell/src/providers/ packages/app-shell/src/services/MetadataService.saveAdvisories.test.ts scripts/__tests__/one-authority-per-exported-name-6273.test.tsnode scripts/check-changeset-presence.mjsnode scripts/check-control-bytes.mjsnode scripts/check-governed-queue-guard.mjs --test <the 3 paths>eslint --no-inline-config --format jsonon the two changed source filesDeclared narrowings, so the omissions are visible rather than implied:
packages/app-shell/src/providers/plus the one consumer suite that imports the type, not the whole 281-file package. The change is type-level and erases at build, so it has no runtime reach; the full package run is CI's.projectService, noprojectineslint.config.js, 337 lines, control grep on the same file fires), so this diff cannot move the verdict on any file it does not touch. Repo-widepnpm lintis CI's.Changeset — why empty frontmatter
packages/app-shell/src/**changed, so a changeset is owed. It declares no release: the removed declaration was byte-identical to the one it now re-exports,export type { X } from '…'erases at build, and the name is on no package's published face, so nothing a consumer of@object-ui/app-shellcan observe changed.skip-changesetis deliberately not applied — in this repository the changeset declaration is the mechanism and that label exempts nothing.Out-of-scope finding, filed not fixed
#8203 records what turned up while measuring this one: the same narrowed-
tshape is declared 11 times across 6 packages under 9 different names, and 8 of those are structurally invisible to this gate, which matches on NAME and counts exported declarations only. Filed with thefindingclass label and no grading. ⛔ Nothing inpackages/fieldsother than the untouchedfile-size-guard.tswas read or changed here, per the #8194 fence.⛔ Left as draft on purpose — the PM lands it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code