Uh oh!
There was an error while loading. Please reload this page.
fix(types): drop dead require condition from @object-ui/types exports - #5315
Merged
os-support-ai merged 2 commits intoAug 19, 2026
Merged
Conversation
The root `.` export's `require` condition pointed at `dist/index.cjs`,
which the package's bare-`tsc` build (`"build": "tsc"`, no bundler)
structurally never emits. Verified on a clean rebuild: dist/ contains
zero .cjs files and require.resolve('.../dist/index.cjs') throws
MODULE_NOT_FOUND.
The package declares "type": "module" and is types-first, so ESM-only
is the contract-honest shape. Verified both premises for dropping
rather than adding an emit step: nothing in-repo require()s this
package (zero grep hits), and the one in-repo CJS-format bundle
consumer (packages/vscode-extension, tsup --format cjs with
noExternal: ['@object-ui/types', '@object-ui/core']) builds
byte-identically (32.87 KB) with the condition present or absent —
esbuild resolves the value imports it re-exports (ACTION_LOCATIONS,
errorCodeIs, actionRendersAt, etc.) via the "import" condition
regardless of its own output format.
Adds a manifest-level pin (packages/types/src/__tests__/
package-exports-manifest.test.ts) so the exports map cannot silently
regress to declaring a condition the build cannot back.Judged non-breaking: measured require('@object-ui/types') through the
real workspace symlink both before and after the fix — it throws
either way (MODULE_NOT_FOUND before, ERR_PACKAGE_PATH_NOT_EXPORTED
after). No working require() call is turned into a failing one.Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 19, 2026 13:21
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-4896-types-exports-require-condition
branch
August 19, 2026 13:22
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#4896
The defect
packages/types/package.json'sexports["."]map declared arequirecondition pointing atdist/index.cjs— a file the package's"build": "tsc"script (baretsc, no bundler, no dual-format step) structurally never emits. Verified on a clean rebuild (rm -rf dist tsconfig.tsbuildinfo && tscinsidepackages/types): zero.cjsfiles underdist/, andrequire.resolve('.../dist/index.cjs')throwsMODULE_NOT_FOUND.Route taken — drop the condition, verified rather than assumed
The triage comment suggested dropping the
requirecondition (ESM-only is the contract-honest shape, given"type": "module"and no bundler) rather than adding a second build format. Both premises behind that route were verified directly, not taken on faith:"type": "module"is declared —packages/types/package.json:5.Nothing in-repo consumes this package via
require()—grep -rn "require(['\"]@object-ui/types"across the whole repo (excludingnode_modules) returns zero hits. Counter-probed with a known-present neighbouring term (require('@object-ui/typesbroadly, and CJSpackage.jsons repo-wide) to make sure the zero was a real zero and not a grep miss: exactly two workspace packages default to CommonJS (packages/vscode-extension,apps/site);apps/sitedeclares@object-ui/typesas a dependency but has zero source references to it (phantom/unused);packages/vscode-extensionis the one real case — it builds viatsup --format cjswithnoExternal: ['@object-ui/types', '@object-ui/core'], forcing both to be bundled into its CJS output, and@object-ui/coredoes value-import runtime exports from@object-ui/types(ACTION_LOCATIONS,errorCodeIs,actionRendersAt, etc. — not just types). I built it both ways to settle whether esbuild's bundler actually needs therequirecondition for this path:Byte-identical output either way — esbuild resolves the
importstatements it's bundling via theimportcondition regardless of its own output format, so therequirecondition was dead weight even for the one real CJS-bundling consumer in this repo.The changeset call
Judged non-breaking →
patch, and here is why, measured rather than asserted. I ran a realrequire('@object-ui/types')through the package's actual workspace symlink (packages/core/node_modules/@object-ui/types, which resolves to../../../types) in both states:MODULE_NOT_FOUND: Cannot find module '.../dist/index.cjs'(the condition existed but pointed nowhere real).ERR_PACKAGE_PATH_NOT_EXPORTED: No "exports" main defined ...(no matching condition).Both throw. No working
require()call is turned into a failing one, because there was never a working one — the build has never emitteddist/index.cjs, in this repo or in any published version. Stayspatchon@object-ui/typesonly — nevermajor, well inside objectui's fixed-group minor/patch lane.The pin — manifest-level, not artifact-level, and why
Added
packages/types/src/__tests__/package-exports-manifest.test.ts: pins the.export to exactly{types, import}, pins"type": "module", pins"build": "tsc"(the premise that makes arequirecondition suspect at all — no bundler, no dual-emit), and asserts no export condition anywhere in the map targets a.cjsfile.This is deliberately a manifest-level pin, not a build-artifact-level gate, for a repo-specific reason:
turbo.json'stesttask onlydependsOn: ["^build"](the dependency closure, never the package's own build), andci.yml's per-PRTestjob runspnpm testwith no build step ahead of it at all. A test that required a freshdist/to exist would be vacuously absent-or-red on a cold CI cache — exactly the trapscripts/check-package-self-import.mjs's header documents for a different gate, and the same reason this repo's own artifact-level gates (check:published-dist,check:node-esm-load's full/LOAD leg) are deliberately release-time/nightly workflows, never per-PR (published-dist-gate.yml,node-esm-load-gate.yml).Reverse-verified, with the fix committed first so the revert is a real restore point:
packages/types/package.jsontoorigin/main's content (test file kept).{types, import}" and "no.cjstarget" tests go red; the "type: module" and "build: tsc" tests stay green.Tests 2 failed | 2 passed (4), both failures naming the reintroducedrequire: ./dist/index.cjs.The artifact-level claim (a clean
tscbuild never emits.cjs) is the one manual proof above, run once for this fix — it doesn't need a repeating gate, because what the pin protects going forward is the manifest staying internally consistent with the build config ("build": "tsc"), and any future change to either would need to touch this test.Out of scope, filed separately if warranted
Per the card: the 22-package exports-vs-dist sweep the original finding (#4896) mentioned is explicitly out of scope for this PR. This fix doesn't establish that the wider pattern is a live problem elsewhere (the two other spot-checked packages in the finding,
data-objectstackandfields, both have build configs that plausibly back theirrequireconditions), so I have not filed a follow-up sweep issue — filing one on top of an unconfirmed pattern would be speculative, not evidence-based.Verification
pnpm exec vitest run packages/types/→ 39 files / 447 tests passed (includes the new pin, in both states above).pnpm --filter '@object-ui/types' type-check→ passed (tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json).node scripts/check-changeset-no-major.mjs→ passed.node scripts/check-changeset-presence.mjs→ passed (1 changeset for 1 released package touched).node scripts/check-phantom-dependencies.mjs→ passed (40 packages, 14925 specifiers scanned).node scripts/check-node-esm-load.mjs --specifiers-only→ passed (cheap/source leg; the build-dependent LOAD leg is the dedicated nightly workflow noted above, not a per-PR gate in this repo).node scripts/check-control-bytes.mjs→ passed.0097d2752.Generated by Claude Code