Skip to content

fix(publish): apply publishConfig manifest overrides under npm publish#5338

Open
NathanFlurry wants to merge 1 commit into
mainfrom
effect-pkg
Open

fix(publish): apply publishConfig manifest overrides under npm publish#5338
NathanFlurry wants to merge 1 commit into
mainfrom
effect-pkg

Conversation

@NathanFlurry

@NathanFlurry NathanFlurry commented Jun 25, 2026

Copy link
Copy Markdown
Member

Problem

Every published version of @rivetkit/effect has its top-level exports["."] pointing at ./src/mod.ts (raw TS source) instead of ./dist. The package's package.json intends the dist path via publishConfig.exports, but that override never lands in the published tarball.

Root cause: publishConfig manifest overrides like exports are effectively a pnpm feature. pnpm rewrites the manifest when it packs the tarball; plain npm publish ignores them. Our CI publishes with npm publish (scripts/publish/src/lib/npm.ts spawns npm publish per package), so the dev src entry ships instead of the dist swap. All published versions of @rivetkit/effect confirm this (exports["."] = "./src/mod.ts").

Impact

Runtime in Bun/Deno/tsx is fine (they execute .ts directly) and bundlers don't deep-type-check deps, so most consumers never see it. It breaks consumers running a strict whole-program tsc --noEmit: under moduleResolution: bundler the entry resolves to our src/*.ts, which then gets compiled under the consumer's tsconfig and errors. skipLibCheck doesn't help (it skips .d.ts, not .ts reached via exports), and the shipped dist/*.d.ts is never used.

Thanks to the external consumer who reported this (their workaround was a bun patch rewriting exports to the dist conditional).

Fix

Make the publishConfig → dist swap actually apply at publish time, centrally, so it works under npm publish and covers any future @rivetkit/* package using the same pattern.

  1. scripts/publish/src/lib/version.tsbumpPackageJsons (the publish-time, non-versionOnly rewrite that CI runs on disk immediately before npm publish) now folds manifest-shape publishConfig fields (exports, main, types, bin, ...) into the top-level manifest, mirroring pnpm's behavior. npm-native publish controls (access, registry, tag, provenance, ...) are intentionally left in publishConfig so npm still reads them.

  2. rivetkit-typescript/packages/effect/package.json — make publishConfig.exports an explicit conditional with a types condition ({ "types": "./dist/mod.d.ts", "default": "./dist/mod.js" }) so strict-tsc consumers resolve the shipped declarations.

This preserves the effect package's deliberate .ts-native dev experience (source exports in the working tree, rewriteRelativeImportExtensions, @effect/language-service) while ensuring the published tarball ships the dist entry.

Why not pnpm publish or a per-package prepack?

Switching the whole publish path to pnpm publish is a broad, riskier infra change (different workspace-dep handling). A prepack script would have to be added per package and mutate package.json on disk with a matching postpack restore. The publish system already rewrites every manifest on disk at publish time (non-committed), so folding publishConfig there is the smallest, most robust hook and generalizes to all packages.

Verification

Replicated the fold against the real effect manifest. Published exports becomes { ".": { "types": "./dist/mod.d.ts", "default": "./dist/mod.js" } } and publishConfig correctly retains only { "access": "public" }. The full-mode bump runs after the TS build step in publish.yaml, so dist/mod.js / dist/mod.d.ts exist when the manifest is finalized.

@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

PR Review

This PR fixes a real publish bug: @rivetkit/effect shipped raw .ts source in its exports entry because npm publish ignores publishConfig (a pnpm-only feature). The fix adds a central applyPublishConfigOverrides step in the publish pipeline that folds manifest-shape publishConfig fields into the top-level manifest before packing, mirroring pnpm's behavior. The effect/package.json change also upgrades the publishConfig.exports entry from a plain string to a types/default conditional so strict-tsc consumers resolve the shipped .d.ts.

Three findings worth addressing:


scripts/publish/src/lib/version.tsPUBLISH_CONFIG_MANIFEST_FIELDS is missing entries that pnpm actually hoists

The repo's allowlist omits "type", "engines", "imports", "os", "cpu", and "libc" — all of which pnpm's own PUBLISH_CONFIG_WHITELIST covers. If a future package sets publishConfig.type (e.g. to switch from "commonjs" to "module" in the published tarball) or uses publishConfig.engines, the field will be silently left in publishConfig and ignored by npm, publishing the wrong value with no warning. The fix is to add the missing entries to match pnpm's set:

 const PUBLISH_CONFIG_MANIFEST_FIELDS = [
     "bin", "main", "module", "exports", "types", "typings",
-    "browser", "esnext", "es2015", "unpkg", "umd:main", "typesVersions",
+    "browser", "esnext", "es2015", "unpkg", "umd:main", "typesVersions",
+    "type", "engines", "imports", "os", "cpu", "libc",
 ] as const;

scripts/publish/src/lib/version.ts line ~40 — [key: string]: unknown weakens PackageJson type safety

Adding the index signature to the interface is necessary for pkgJson[field] = publishConfig[field] to compile, but it also silently accepts any misspelled key (pkgJson["dependenciez"] = ...) without error. The named property types still win for the DEP_FIELDS computed-access loop (TypeScript 5.5 resolves those correctly), but future callers lose compile-time protection on unknown keys. A narrower fix is to keep the interface clean and cast only at the mutation site:

(pkgJson as Record<string, unknown>)[field] = publishConfig[field];

This leaves the PackageJson interface fully typed and removes the need for [key: string]: unknown entirely.


scripts/publish/src/lib/version.ts line ~183 — fold log fires without [dry-run] prefix

The rest of the dry-run path logs [dry-run] would update ${pkg.name} -> ${version}, but the fold log (folded publishConfig into ${pkg.name}: exports) fires unconditionally before the dry-run check with no [dry-run] marker. An operator running --dry-run sees the fold message as if it were a live side-effect. Wrapping it in the same opts.dryRun guard (or prefixing with [dry-run]) would keep the output consistent.


The core logic is correct — the publishConfig deletion guard (Object.keys(publishConfig).length === 0) properly leaves npm-native fields like access in place, and the effect types/default conditional export is the right shape for strict-tsc consumers. The missing allowlist entries are the most likely to bite in practice.

@railway-app

railway-app Bot commented Jun 25, 2026

Copy link
Copy Markdown

🚅 Deployed to the rivet-pr-5338 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-cloud 😴 Sleeping (View Logs) Web Jun 25, 2026 at 8:48 pm
kitchen-sink 😴 Sleeping (View Logs) Web Jun 25, 2026 at 8:48 pm
frontend-inspector 😴 Sleeping (View Logs) Web Jun 25, 2026 at 8:48 pm
website 😴 Sleeping (View Logs) Web Jun 25, 2026 at 8:48 pm
ladle ✅ Success (View Logs) Web Jun 25, 2026 at 8:38 pm
mcp-hub ✅ Success (View Logs) Web Jun 25, 2026 at 8:38 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant