build: migrate packages to tsdown - #1206
edmundhung wants to merge 2 commits into
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrates six packages from Rollup+tsc to tsdown: adds root Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d2ef3c6 to
c6739a7
Compare
Deploying conform with
|
| Latest commit: |
625c24b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e51b64e0.conform.pages.dev |
| Branch Preview URL: | https://tsdown-future.conform.pages.dev |
More templates
@conform-to/dom
@conform-to/react
@conform-to/valibot
@conform-to/validitystate
@conform-to/yup
@conform-to/zod
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-react/tsdown.config.mjs`:
- Around line 12-20: The prependUseClient function currently only checks for the
exact string "'use client';" and will miss variants (double quotes, missing
semicolon, leading whitespace or BOM); update prependUseClient to detect an
existing directive using a regex that tolerates an optional UTF-8 BOM, optional
leading whitespace, either single or double quotes around use client, and an
optional semicolon (e.g. match /^\uFEFF?\s*(['"])use client\1\s*;?/), return
early if it matches, and keep the existing behavior of writing a canonical `'use
client';\n` prefix when no match is found.
- Around line 46-56: The build hook 'build:done' currently iterates chunks and
silently skips files not in the useClientEntries Set, which can result in
missing 'use client' directives; update the hook (build:done) to validate
post-condition by either (a) tracking which expected filenames from
useClientEntries were actually processed and throwing or logging an error if any
expected file (e.g. future/forms.mjs, future/forms.js, future/hooks.mjs,
future/hooks.js) was not prepended via prependUseClient, or (b) instead revert
to file-system validation by checking fs.existsSync(path.join('dist',
expectedFile)) for each expected entry and calling prependUseClient for any
existing files, failing the build if any expected file is missing; ensure you
reference useClientEntries, chunks, and prependUseClient in the change so the
hook fails fast when the four expected files are not handled.
In `@packages/conform-validitystate/tsdown.config.mjs`:
- Around line 1-27: Extract a shared factory (e.g., createTsdownConfig({ entry,
hooks })) at the repo root and replace the duplicate package-specific
tsdown.config.mjs files to call that factory; move common options (platform,
target, unbundle, deps.skipNodeModulesBundle, copy, outExtensions, dts, clean,
format, root, report, outDir defaults) into the factory and keep per-package
overrides (entry, tsconfig, hooks) passed in, then update each package's
tsdown.config.mjs (including the config in the current file and those in
conform-dom, conform-react, conform-valibot, conform-yup, conform-zod) to import
and invoke createTsdownConfig with their package-specific fields.
In `@packages/conform-zod/tsdown.config.mjs`:
- Around line 3-33: The tsdown config is duplicated across packages; refactor by
creating a shared factory function (e.g., scripts/tsdown.config.base.mjs) that
exports a function which accepts the unique entry array and returns the common
config object (the object currently passed to defineConfig in this file), then
update this file to call that factory and pass its entry; preserve keys like
entry, tsconfig, dts, clean, outDir, unbundle, format, platform, target, report,
deps, copy, and the outExtensions behavior (js => '.mjs' for esm/es), and
continue to wrap the returned object with defineConfig if required.
- Line 17: The format array currently lists ['esm','cjs'] which will emit .js
files; make the build emit explicit .mjs/.cjs extensions to avoid breakage if
package "type" changes by enabling the bundler option fixedExtension: true in
tsdown.config.mjs (and keep format: ['esm','cjs']), or alternatively change the
format entries to explicitly map to .mjs/.cjs output; locate the format array
and the tsdown config object and add/flip the fixedExtension: true flag (or
adjust format mapping) so outputs use .mjs for ESM and .cjs for CJS.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 222f56d2-9f74-442e-8f93-649f4d6b7338
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
examples/radix-ui/package.jsonexamples/react-router/package.jsonexamples/react-spa/package.jsonexamples/shadcn-ui/package.jsonpackage.jsonpackages/conform-dom/package.jsonpackages/conform-dom/rollup.config.jspackages/conform-dom/tsconfig.build.jsonpackages/conform-dom/tsdown.config.mjspackages/conform-react/package.jsonpackages/conform-react/rollup.config.jspackages/conform-react/tsconfig.build.jsonpackages/conform-react/tsdown.config.mjspackages/conform-valibot/package.jsonpackages/conform-valibot/rollup.config.jspackages/conform-valibot/tsconfig.build.jsonpackages/conform-valibot/tsdown.config.mjspackages/conform-validitystate/package.jsonpackages/conform-validitystate/rollup.config.jspackages/conform-validitystate/tsconfig.build.jsonpackages/conform-validitystate/tsdown.config.mjspackages/conform-yup/package.jsonpackages/conform-yup/rollup.config.jspackages/conform-yup/tsconfig.build.jsonpackages/conform-yup/tsdown.config.mjspackages/conform-zod/package.jsonpackages/conform-zod/rollup.config.jspackages/conform-zod/tsconfig.build.jsonpackages/conform-zod/tsdown.config.mjsplayground/package.json
💤 Files with no reviewable changes (12)
- packages/conform-validitystate/tsconfig.build.json
- packages/conform-yup/tsconfig.build.json
- packages/conform-valibot/tsconfig.build.json
- packages/conform-zod/tsconfig.build.json
- packages/conform-dom/tsconfig.build.json
- packages/conform-validitystate/rollup.config.js
- packages/conform-react/rollup.config.js
- packages/conform-react/tsconfig.build.json
- packages/conform-dom/rollup.config.js
- packages/conform-valibot/rollup.config.js
- packages/conform-zod/rollup.config.js
- packages/conform-yup/rollup.config.js
| async function prependUseClient(filePath) { | ||
| const content = await fs.readFile(filePath, 'utf8'); | ||
|
|
||
| if (content.startsWith("'use client';")) { | ||
| return; | ||
| } | ||
|
|
||
| await fs.writeFile(filePath, `'use client';\n${content}`); | ||
| } |
There was a problem hiding this comment.
Minor: 'use client'; check should tolerate double quotes and BOM.
content.startsWith("'use client';") only recognises the exact single-quoted form with a trailing semicolon and no leading whitespace/BOM. If any upstream tool (including a future tsdown change) emits "use client";, 'use client' without the semicolon, or prepends a UTF-8 BOM, you'll stack a duplicate directive on every rebuild. Low likelihood today, but cheap to make robust:
- if (content.startsWith("'use client';")) {
+ const stripped = content.replace(/^\uFEFF/, '').trimStart();
+ if (/^['"]use client['"]\s*;?/.test(stripped)) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function prependUseClient(filePath) { | |
| const content = await fs.readFile(filePath, 'utf8'); | |
| if (content.startsWith("'use client';")) { | |
| return; | |
| } | |
| await fs.writeFile(filePath, `'use client';\n${content}`); | |
| } | |
| async function prependUseClient(filePath) { | |
| const content = await fs.readFile(filePath, 'utf8'); | |
| const stripped = content.replace(/^\uFEFF/, '').trimStart(); | |
| if (/^['"]use client['"]\s*;?/.test(stripped)) { | |
| return; | |
| } | |
| await fs.writeFile(filePath, `'use client';\n${content}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-react/tsdown.config.mjs` around lines 12 - 20, The
prependUseClient function currently only checks for the exact string "'use
client';" and will miss variants (double quotes, missing semicolon, leading
whitespace or BOM); update prependUseClient to detect an existing directive
using a regex that tolerates an optional UTF-8 BOM, optional leading whitespace,
either single or double quotes around use client, and an optional semicolon
(e.g. match /^\uFEFF?\s*(['"])use client\1\s*;?/), return early if it matches,
and keep the existing behavior of writing a canonical `'use client';\n` prefix
when no match is found.
| hooks: { | ||
| async 'build:done'({ chunks }) { | ||
| for (const chunk of chunks) { | ||
| if (!useClientEntries.has(chunk.fileName)) { | ||
| continue; | ||
| } | ||
|
|
||
| await prependUseClient(path.join('dist', chunk.fileName)); | ||
| } | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Check the installed tsdown version(s) and the hook type
fd -t f 'package.json' -d 3 --exec grep -lH '"tsdown"' {} \;
rg -nP --type=json '"tsdown"\s*:\s*"' package.json pnpm-workspace.yaml 2>/dev/null
# 2) Inspect the tsdown hook type definition from the installed package, if present
fd -t f 'hooks.d.ts' node_modules/tsdown 2>/dev/null --exec sed -n '1,120p' {} \;
fd -t f 'hooks.ts' node_modules/tsdown 2>/dev/null --exec sed -n '1,120p' {} \;Repository: edmundhung/conform
Length of output: 117
🌐 Web query:
tsdown release notes "build:done" chunks added
💡 Result:
tsdown’s release-related change that “build:done” chunks were added is documented in the changelog/release notes for v0.17.0 as “Add chunks on build:done hook”. What changed - The build:done hook handler signature was updated to receive the built Rolldown chunks (i.e., the hook context now includes chunks). - In code, build:done is invoked as hooks.callHook('build:done', { ...context, chunks }). Where it appears - v0.17.0 release notes: “Add chunks on build:done hook”. - The underlying implementation is in the commit “feat: add chunks on build:done hook” and the hook type definition (TsdownHooks) and buildSingle call site. (So the search term you gave matches a real tsdown change, implemented by commit eb45c79 and released in v0.17.0.)
Citations:
- 1: https://github.com/rolldown/tsdown/releases/tag/v0.17.0
- 2: rolldown/tsdown@eb45c79
- 3: https://github.com/rolldown/tsdown/blob/main/src/features/hooks.ts
- 4: https://github.com/rolldown/tsdown/blob/cd24d3e407ff048433614cb6e48ea9dc5523a09c/src/build.ts
🏁 Script executed:
# Read the tsdown.config.mjs file to see full context
cd packages/conform-react && wc -l tsdown.config.mjs && cat -n tsdown.config.mjsRepository: edmundhung/conform
Length of output: 1659
🏁 Script executed:
# Find the old rollup.config.js to compare implementations
fd -t f 'rollup.config.js' --type fRepository: edmundhung/conform
Length of output: 44
🏁 Script executed:
# Check how useClientEntries is populated and what entries are expected
rg -n 'useClientEntries' packages/conform-react/tsdown.config.mjs -B 5 -A 10Repository: edmundhung/conform
Length of output: 752
🏁 Script executed:
# Search for rollup.config.js to compare old implementation
find . -name 'rollup.config.js' -o -name 'rollup.config.mjs' | head -20Repository: edmundhung/conform
Length of output: 44
🏁 Script executed:
# Check if there are any tests or validation for the 'use client' directive
rg -l "use client" --type js --type ts --type mjs --type mts packages/conform-react/ | head -10Repository: edmundhung/conform
Length of output: 90
🏁 Script executed:
# Look for any GitHub issues or docs mentioning the tsdown build:done hook and chunks
rg -i "build:done" . --type md --type json --type ts --type js | head -20Repository: edmundhung/conform
Length of output: 44
🏁 Script executed:
# Search git history for rollup config or previous implementation
git log --all --oneline --follow --name-only -- '*rollup*' | head -30Repository: edmundhung/conform
Length of output: 1525
🏁 Script executed:
# Check git diff or commit history for tsdown.config.mjs to see what changed
git log --oneline -n 20 -- packages/conform-react/tsdown.config.mjsRepository: edmundhung/conform
Length of output: 104
🏁 Script executed:
# Verify the actual chunk filenames that would be generated
# by inspecting the entry config and format settings
rg -n "entry|format|unbundle" packages/conform-react/tsdown.config.mjs -A 2Repository: edmundhung/conform
Length of output: 380
🏁 Script executed:
# Check if there are any recent issues or TODOs in the code about this
rg -n "TODO|FIXME|XXX" packages/conform-react/tsdown.config.mjsRepository: edmundhung/conform
Length of output: 44
🏁 Script executed:
# Find and examine the old rollup.config.js to compare approaches
git show 3396b6a:packages/conform-react/rollup.config.js | head -100Repository: edmundhung/conform
Length of output: 2386
🏁 Script executed:
# Also check the commit message for context
git log -1 3396b6a --format="%B"Repository: edmundhung/conform
Length of output: 128
🏁 Script executed:
# Verify what chunks would actually be generated by looking at the source
ls -la packages/conform-react/future/Repository: edmundhung/conform
Length of output: 779
🏁 Script executed:
# Check if there are any tests verifying the 'use client' directive is applied
find . -path ./node_modules -prune -o -name '*.test.*' -o -name '*.spec.*' | xargs grep -l "use client" 2>/dev/null | head -10Repository: edmundhung/conform
Length of output: 44
🏁 Script executed:
# Check the git commit that migrated to tsdown to understand context
git log --oneline --all | grep -i tsdown | head -5Repository: edmundhung/conform
Length of output: 104
🏁 Script executed:
# Examine what the current dist output looks like after a build (if available)
ls -la packages/conform-react/dist/future/ 2>/dev/null || echo "dist not available in sandbox"Repository: edmundhung/conform
Length of output: 92
🏁 Script executed:
# Look at the tsdown version again and check if there's any documentation in node_modules
cat packages/conform-react/package.json | grep -A 5 -B 5 tsdownRepository: edmundhung/conform
Length of output: 398
Remove silent-failure vulnerability: assert all expected 'use client' files are actually prepended.
The implementation correctly relies on chunks from the build:done hook (available since tsdown v0.17.0, and the project uses v0.21.10). However, there is no post-condition validation: if a chunk filename does not match any entry in the useClientEntries Set, the loop succeeds silently without prepending 'use client' to any files. Unlike the previous rollup implementation which checked fs.existsSync() directly, the current approach depends entirely on exact chunk metadata matching.
This creates a risk: if output naming shifts (e.g., due to unbundle path layout changes or future tsdown versions), the four expected files (future/{forms,hooks}.{mjs,js}) will be silently skipped, and the published package will lack the RSC directive—breaking consumers' Server Component builds.
Suggested fix: assert that all four expected files were prepended, or revert to direct fs.existsSync validation:
Suggested hardening
hooks: {
async 'build:done'({ chunks }) {
const results = await Promise.all(
[...useClientEntries].map(async (entry) => {
const filePath = path.join('dist', entry);
try {
await prependUseClient(filePath);
return entry;
} catch (err) {
if (err?.code === 'ENOENT') return null;
throw err;
}
}),
);
const prepended = results.filter(Boolean);
if (prepended.length === 0) {
throw new Error(
"tsdown build:done — no 'use client' targets were found in dist/. " +
'Check entry configuration or output naming.',
);
}
},
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-react/tsdown.config.mjs` around lines 46 - 56, The build
hook 'build:done' currently iterates chunks and silently skips files not in the
useClientEntries Set, which can result in missing 'use client' directives;
update the hook (build:done) to validate post-condition by either (a) tracking
which expected filenames from useClientEntries were actually processed and
throwing or logging an error if any expected file (e.g. future/forms.mjs,
future/forms.js, future/hooks.mjs, future/hooks.js) was not prepended via
prependUseClient, or (b) instead revert to file-system validation by checking
fs.existsSync(path.join('dist', expectedFile)) for each expected entry and
calling prependUseClient for any existing files, failing the build if any
expected file is missing; ensure you reference useClientEntries, chunks, and
prependUseClient in the change so the hook fails fast when the four expected
files are not handled.
| import { defineConfig } from 'tsdown'; | ||
|
|
||
| export default defineConfig({ | ||
| entry: ['index.ts'], | ||
| tsconfig: './tsconfig.json', | ||
| dts: true, | ||
| clean: true, | ||
| outDir: 'dist', | ||
| root: '.', | ||
| unbundle: true, | ||
| format: ['esm', 'cjs'], | ||
| platform: 'neutral', | ||
| target: 'node16', | ||
| report: false, | ||
| deps: { | ||
| skipNodeModulesBundle: true, | ||
| }, | ||
| copy: [ | ||
| { from: '../../LICENSE', to: '.' }, | ||
| { from: '../../README.md', to: '.' }, | ||
| ], | ||
| outExtensions({ format }) { | ||
| return { | ||
| js: format === 'esm' || format === 'es' ? '.mjs' : '.js', | ||
| }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: centralize the shared tsdown config.
This file is nearly identical to packages/conform-dom/tsdown.config.mjs, packages/conform-react/tsdown.config.mjs, and the configs added for conform-valibot, conform-yup, and conform-zod. Consider extracting a createTsdownConfig({ entry, hooks }) helper at the repo root so options like platform, target, unbundle, deps, copy, and outExtensions stay in sync across packages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-validitystate/tsdown.config.mjs` around lines 1 - 27,
Extract a shared factory (e.g., createTsdownConfig({ entry, hooks })) at the
repo root and replace the duplicate package-specific tsdown.config.mjs files to
call that factory; move common options (platform, target, unbundle,
deps.skipNodeModulesBundle, copy, outExtensions, dts, clean, format, root,
report, outDir defaults) into the factory and keep per-package overrides (entry,
tsconfig, hooks) passed in, then update each package's tsdown.config.mjs
(including the config in the current file and those in conform-dom,
conform-react, conform-valibot, conform-yup, conform-zod) to import and invoke
createTsdownConfig with their package-specific fields.
| export default defineConfig({ | ||
| entry: [ | ||
| 'default/index.ts', | ||
| 'v3/index.ts', | ||
| 'v3/future.ts', | ||
| 'v4/index.ts', | ||
| 'v4/future.ts', | ||
| ], | ||
| tsconfig: './tsconfig.json', | ||
| dts: true, | ||
| clean: true, | ||
| outDir: 'dist', | ||
| root: '.', | ||
| unbundle: true, | ||
| format: ['esm', 'cjs'], | ||
| platform: 'neutral', | ||
| target: 'node16', | ||
| report: false, | ||
| deps: { | ||
| skipNodeModulesBundle: true, | ||
| }, | ||
| copy: [ | ||
| { from: '../../LICENSE', to: '.' }, | ||
| { from: '../../README.md', to: '.' }, | ||
| ], | ||
| outExtensions({ format }) { | ||
| return { | ||
| js: format === 'esm' || format === 'es' ? '.mjs' : '.js', | ||
| }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting a shared tsdown config factory.
Per the PR summary, six packages receive nearly identical tsdown configs (only entry differs). Duplicating 25+ lines across packages makes future build-flag changes (target bump, CJS extension fix, copy list, etc.) a six-place edit. A small shared helper (e.g. scripts/tsdown.config.base.mjs) accepting entry and returning a config object would remove the duplication without changing behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-zod/tsdown.config.mjs` around lines 3 - 33, The tsdown
config is duplicated across packages; refactor by creating a shared factory
function (e.g., scripts/tsdown.config.base.mjs) that exports a function which
accepts the unique entry array and returns the common config object (the object
currently passed to defineConfig in this file), then update this file to call
that factory and pass its entry; preserve keys like entry, tsconfig, dts, clean,
outDir, unbundle, format, platform, target, report, deps, copy, and the
outExtensions behavior (js => '.mjs' for esm/es), and continue to wrap the
returned object with defineConfig if required.
| outDir: 'dist', | ||
| root: '.', | ||
| unbundle: true, | ||
| format: ['esm', 'cjs'], |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect each migrated package.json to confirm the "type" field is compatible with CJS emitted as .js.
fd -HI '^package\.json$' packages --max-depth 2 \
--exec sh -c 'echo "=== {} ==="; jq "{name, type, main, module, exports}" "{}"'Repository: edmundhung/conform
Length of output: 1482
🏁 Script executed:
#!/bin/bash
# Fix the jq syntax and inspect package.json files
fd -HI '^package\.json$' packages --max-depth 2 \
--exec sh -c 'echo "=== {} ==="; jq ".name, .type, .main, .module, .exports" "{}"'Repository: edmundhung/conform
Length of output: 3364
🏁 Script executed:
# Read the tsdown.config.mjs to see the full configuration, especially lines 17-32
cat -n packages/conform-zod/tsdown.config.mjsRepository: edmundhung/conform
Length of output: 902
Consider using .mjs/.cjs extensions for better robustness.
The current package configurations have "type" unset (defaulting to CommonJS), so the .js CJS output is currently safe. However, this pattern is fragile — if someone adds "type": "module" to any package.json, Node will misinterpret .js CJS files as ESM and fail at require-time. The conventional safe pairing is .mjs for ESM and .cjs for CJS (which fixedExtension: true produces):
Recommended change
- outExtensions({ format }) {
- return {
- js: format === 'esm' || format === 'es' ? '.mjs' : '.js',
- };
- },
+ fixedExtension: true, // emits .mjs for ESM and .cjs for CJS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-zod/tsdown.config.mjs` at line 17, The format array
currently lists ['esm','cjs'] which will emit .js files; make the build emit
explicit .mjs/.cjs extensions to avoid breakage if package "type" changes by
enabling the bundler option fixedExtension: true in tsdown.config.mjs (and keep
format: ['esm','cjs']), or alternatively change the format entries to explicitly
map to .mjs/.cjs output; locate the format array and the tsdown config object
and add/flip the fixedExtension: true flag (or adjust format mapping) so outputs
use .mjs for ESM and .cjs for CJS.
c6739a7 to
da787ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/conform-react/package.json (1)
26-29:⚠️ Potential issue | 🟠 MajorExtend
filesglob to include.d.mtsand.d.ctsdeclaration files.tsdown with
format: ['esm', 'cjs']anddts: trueemits.d.mts(for ESM) and.d.cts(for CJS) declaration files, but the current glob./dist/**/*.d.tsonly captures plain.d.tsfiles, silently excluding the format-specific declarations from the published tarball.This affects all six packages (conform-dom, conform-react, conform-yup, conform-valibot, conform-validitystate, conform-zod), which all use identical
filesglobs. Update the glob to./dist/**/*.d.{ts,mts,cts}or similar to ensure all generated declaration files are included.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-react/package.json` around lines 26 - 29, The package "files" globs currently only include "./dist/**/*.d.ts" which misses tsdown-produced .d.mts and .d.cts declaration files; update the "files" array entry that contains "./dist/**/*.d.ts" in each package.json (e.g., in conform-react and the other five packages) to a glob that includes format-specific declarations such as "./dist/**/*.d.{ts,mts,cts}" (or add separate entries for "./dist/**/*.d.mts" and "./dist/**/*.d.cts") so all generated .d.* declaration files are packaged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/react-spa/package.json`:
- Line 16: The package.json downgrade removed the previously-upgraded
react-router versions; update the dependency entries for "react-router",
"@react-router/node", "@react-router/serve", and "@react-router/dev" to the
intended version ^7.14.2 (or alternatively add a clear comment/PR description
explaining why the downgrade to ^7.8.2 is intentional), ensuring the changes are
applied consistently in both examples/react-router and examples/react-spa
dependency lists and the commit message documents the rationale if keeping the
older version.
In `@examples/shadcn-ui/package.json`:
- Line 50: The PR unintentionally downgrades the postcss dependency across
example/playground package.json files, causing inconsistent versions; update the
"postcss" entries in examples/radix-ui/package.json,
examples/shadcn-ui/package.json, and playground/package.json to the same
agreed-upon version (e.g. ^8.5.10) so they are standardized across the
workspace, commit the synchronized changes, and re-run install to ensure the
lockfile is consistent.
In `@packages/conform-react/tsdown.config.mjs`:
- Around line 5-10: The hardcoded const useClientEntries set will drift from the
bundler's format/outExtensions, causing the client directive to not be
prepended; change the logic that builds useClientEntries to derive entries
dynamically from the project's format and outExtensions (e.g., iterate
formats/outExtensions and generate 'future/forms' and 'future/hooks' with each
extension) so it always matches produced chunk filenames; update any references
to useClientEntries to use the newly generated Set and ensure behavior is
identical when formats include or exclude CJS/ESM extensions.
- Line 24: Update each package's tsconfig.json (including the one referenced by
tsconfig: './tsconfig.json' in tsdown.config.mjs and the packages conform-dom,
conform-yup, conform-zod, conform-valibot, conform-validitystate, and
conform-react) to include an "exclude" array with ["**/tests", "dist"];
specifically, open each package's tsconfig.json and add the property exclude:
["**/tests", "dist"] at the top level so test fixtures and built dist files are
not picked up by TypeScript/dts generation.
In `@packages/conform-valibot/tsdown.config.mjs`:
- Around line 1-27: Extract shared tsdown options into a factory (e.g.,
createTsdownConfig) at repo root that returns the common config pieces
(platform, target, unbundle, deps.skipNodeModulesBundle, copy, outExtensions)
and accept overrides like entry and hooks; replace this file's defineConfig call
with createTsdownConfig({ entry: ['index.ts','future.ts'], ...overrides }) and
update the other packages to use the same factory so options stay in sync. While
extracting, simplify outExtensions in the factory to only check format === 'esm'
(drop the unreachable 'es' branch) and ensure per-package configs can still
override or extend any fields when needed.
In `@packages/conform-yup/tsdown.config.mjs`:
- Around line 1-27: The tsdown config is duplicated across packages; extract a
shared factory to avoid drift by creating a repo-level helper function (e.g.
createTsdownConfig) that returns defineConfig with the common options (tsconfig,
dts, clean, outDir, root, unbundle, format, platform, target, report, deps,
outExtensions, default copy) and then update this file to import and call that
factory with package-specific overrides for entry, copy and (for react) hooks;
update other package configs similarly so they pass only overrides into
createTsdownConfig instead of duplicating the full defineConfig block.
---
Outside diff comments:
In `@packages/conform-react/package.json`:
- Around line 26-29: The package "files" globs currently only include
"./dist/**/*.d.ts" which misses tsdown-produced .d.mts and .d.cts declaration
files; update the "files" array entry that contains "./dist/**/*.d.ts" in each
package.json (e.g., in conform-react and the other five packages) to a glob that
includes format-specific declarations such as "./dist/**/*.d.{ts,mts,cts}" (or
add separate entries for "./dist/**/*.d.mts" and "./dist/**/*.d.cts") so all
generated .d.* declaration files are packaged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6dd8c3e-c120-4f9b-84c0-59624c40da4a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
examples/radix-ui/package.jsonexamples/react-router/package.jsonexamples/react-spa/package.jsonexamples/shadcn-ui/package.jsonpackage.jsonpackages/conform-dom/package.jsonpackages/conform-dom/rollup.config.jspackages/conform-dom/tsconfig.build.jsonpackages/conform-dom/tsdown.config.mjspackages/conform-react/package.jsonpackages/conform-react/rollup.config.jspackages/conform-react/tsconfig.build.jsonpackages/conform-react/tsdown.config.mjspackages/conform-valibot/package.jsonpackages/conform-valibot/rollup.config.jspackages/conform-valibot/tsconfig.build.jsonpackages/conform-valibot/tsdown.config.mjspackages/conform-validitystate/package.jsonpackages/conform-validitystate/rollup.config.jspackages/conform-validitystate/tsconfig.build.jsonpackages/conform-validitystate/tsdown.config.mjspackages/conform-yup/package.jsonpackages/conform-yup/rollup.config.jspackages/conform-yup/tsconfig.build.jsonpackages/conform-yup/tsdown.config.mjspackages/conform-zod/package.jsonpackages/conform-zod/rollup.config.jspackages/conform-zod/tsconfig.build.jsonpackages/conform-zod/tsdown.config.mjsplayground/package.json
💤 Files with no reviewable changes (12)
- packages/conform-dom/tsconfig.build.json
- packages/conform-react/tsconfig.build.json
- packages/conform-validitystate/tsconfig.build.json
- packages/conform-valibot/tsconfig.build.json
- packages/conform-zod/tsconfig.build.json
- packages/conform-yup/tsconfig.build.json
- packages/conform-validitystate/rollup.config.js
- packages/conform-dom/rollup.config.js
- packages/conform-react/rollup.config.js
- packages/conform-valibot/rollup.config.js
- packages/conform-zod/rollup.config.js
- packages/conform-yup/rollup.config.js
|
|
||
| export default defineConfig({ | ||
| entry: ['index.ts', 'future/index.ts'], | ||
| tsconfig: './tsconfig.json', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare each package's tsconfig.json to what the old tsconfig.build.json referenced.
for pkg in conform-dom conform-react conform-yup conform-zod conform-valibot conform-validitystate; do
echo "===== packages/$pkg/tsconfig.json ====="
cat "packages/$pkg/tsconfig.json" 2>/dev/null || echo "(missing)"
echo
echo "----- prior tsconfig.build.json (from main) -----"
git show "main:packages/$pkg/tsconfig.build.json" 2>/dev/null || echo "(not present on main)"
echo
doneRepository: edmundhung/conform
Length of output: 3770
🏁 Script executed:
# Check if there are base tsconfig.json files that these extend, and their includes/excludes
for pkg in conform-dom conform-react conform-yup conform-zod conform-valibot conform-validitystate; do
echo "===== packages/$pkg/tsconfig files ====="
ls -la "packages/$pkg/tsconfig"* 2>/dev/null || echo "(no tsconfig* files)"
echo
echo "----- Content of tsconfig (parent) if exists -----"
cat "packages/$pkg/tsconfig" 2>/dev/null || echo "(tsconfig not found)"
echo
doneRepository: edmundhung/conform
Length of output: 1324
🏁 Script executed:
# Check if tsconfig.build.json still exists in any package (verify it was deleted)
for pkg in conform-dom conform-react conform-yup conform-zod conform-valibot conform-validitystate; do
if [ -f "packages/$pkg/tsconfig.build.json" ]; then
echo "EXISTS: packages/$pkg/tsconfig.build.json"
fi
done
echo "---"
echo "If no output above, tsconfig.build.json was successfully deleted from all packages"Repository: edmundhung/conform
Length of output: 149
🏁 Script executed:
# Check git diff to see what was changed regarding tsconfig files in this PR
git diff main HEAD --name-only -- "packages/*/tsconfig*" | head -30Repository: edmundhung/conform
Length of output: 324
🏁 Script executed:
# Check for tsconfig at repo root and in parent directories
echo "===== Root tsconfig files ====="
ls -la tsconfig* 2>/dev/null || echo "(none at root)"
echo
# Show root tsconfig.json if it exists
if [ -f tsconfig.json ]; then
echo "Content of root tsconfig.json:"
cat tsconfig.json
echo
fi
# Also search for any tsconfig references in package.json
echo "===== Checking package.json references to tsconfig ====="
grep -A2 -B2 "tsconfig" packages/conform-react/package.json 2>/dev/null || echo "(no tsconfig references)"Repository: edmundhung/conform
Length of output: 550
🏁 Script executed:
# Get the actual full tsconfig.json content for packages to see if they have includes
echo "===== Full tsconfig.json contents (checking for 'include' clauses) ====="
for pkg in conform-dom conform-react conform-yup conform-zod conform-valibot conform-validitystate; do
echo "--- packages/$pkg/tsconfig.json ---"
cat "packages/$pkg/tsconfig.json"
echo
doneRepository: edmundhung/conform
Length of output: 2553
🏁 Script executed:
# Check what files actually exist in each package source directory
echo "===== Source structure in packages ====="
for pkg in conform-dom conform-react conform-yup conform-zod conform-valibot conform-validitystate; do
echo "--- packages/$pkg/ ---"
find "packages/$pkg" -maxdepth 2 -type f -name "*.ts" -o -name "*.tsx" -o -name "*.test.ts" -o -name "*.test.tsx" | head -20
echo
doneRepository: edmundhung/conform
Length of output: 2871
Add exclude: ["**/tests", "dist"] to each package's tsconfig.json.
The current tsconfig.json files for five packages (conform-dom, conform-yup, conform-zod, conform-valibot, conform-validitystate) lack any exclude clause. When TypeScript has no explicit include or exclude, it defaults to **/*, which includes test files. The old tsconfig.build.json excluded **/tests to prevent this. Without restoring this exclusion, tsdown's dts: true pass will type-check and potentially emit .d.ts for test fixtures, and may pull in test-related ambient types (vitest, @types/node test utilities, etc.) into the published type surface.
conform-react partially addresses this by excluding dist, but still includes test files. All six packages should have the **/tests exclusion restored, matching the prior tsconfig.build.json configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-react/tsdown.config.mjs` at line 24, Update each package's
tsconfig.json (including the one referenced by tsconfig: './tsconfig.json' in
tsdown.config.mjs and the packages conform-dom, conform-yup, conform-zod,
conform-valibot, conform-validitystate, and conform-react) to include an
"exclude" array with ["**/tests", "dist"]; specifically, open each package's
tsconfig.json and add the property exclude: ["**/tests", "dist"] at the top
level so test fixtures and built dist files are not picked up by TypeScript/dts
generation.
3ca2a60 to
a978aa6
Compare
a978aa6 to
e634ece
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/conform-react/package.json (1)
26-29:⚠️ Potential issue | 🟠 Major
fileswhitelist drops the.d.ctsdeclarations that tsdown will emit for the CJS build — affects all 6 migrated packages.With
format: ['esm', 'cjs']anddts: true(tsdown's default when TypeScript is installed), tsdown emits a separate CJS declaration pass producing*.d.ctsalongside*.d.ts. The current glob./dist/**/*.d.tsdoes not match*.d.cts, so those files will be generated every build but silently stripped from the published tarball.Two downstream effects:
- Under
moduleResolution: "nodenext" | "node16", CJS consumers whorequire('@conform-to/react')may resolve to the top-level.d.ts(ESM-shaped declarations) rather than a matching.d.cts, which can triggerERR_REQUIRE_ESM-style type warnings inarethetypeswrong.- Every build wastes time emitting files that are never shipped.
Pick one of these remediations (apply to all six
packages/*/package.json):🛠️ Option A — publish both declaration variants and wire them into exports (recommended)
"files": [ - "./dist/**/*.{js,mjs}", - "./dist/**/*.d.ts" + "./dist/**/*.{js,mjs,d.ts,d.cts}" ],And update each entry in
exportsto use conditional types so CJS consumers get.d.cts:"exports": { ".": { - "types": "./dist/index.d.ts", - "module": "./dist/index.mjs", - "import": "./dist/index.mjs", - "require": "./dist/index.js", - "default": "./dist/index.mjs" + "module": "./dist/index.mjs", + "import": { "types": "./dist/index.d.ts", "default": "./dist/index.mjs" }, + "require": { "types": "./dist/index.d.cts", "default": "./dist/index.js" }, + "default": "./dist/index.mjs" },🛠️ Option B — suppress CJS dts generation in `tsdown.config.mjs` if the extra declarations aren't needed
Consult tsdown's dts configuration options to selectively disable CJS-only declaration generation, then ensure the
filesglob matches what remains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-react/package.json` around lines 26 - 29, The package.json files currently only include "./dist/**/*.d.ts" which drops CJS declaration files (*.d.cts) emitted by tsdown; update each packages/*/package.json (all six migrated packages) to include "./dist/**/*.d.cts" in the "files" array and implement Option A: publish both declaration variants and update the "exports" entries to provide conditional paths so CJS consumers resolve to the .d.cts and ESM consumers to .d.ts (adjust each export target for main/module/types to reference the matching .d.cts/.d.ts), or alternatively disable CJS dts output in tsdown.config.mjs if you prefer Option B—apply the chosen fix consistently across all six packages.
♻️ Duplicate comments (1)
packages/conform-zod/package.json (1)
10-50:⚠️ Potential issue | 🟡 MinorHeads-up: the
.d.cts/ conditional-types remediation covers five subpath exports here.This package's
exportsmap has five entries (.,./v3,./v3/future,./v4,./v4/future), each currently using unconditionaltypespointing at.d.ts. If you apply Option A from thepackages/conform-react/package.jsoncomment (publishing both declaration variants with conditionalimport/requiretypes), the edit needs to be repeated for all five subpaths in this file. Option B (disabling CJS dts intsdown.config.mjs) is the smallest delta here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/conform-zod/package.json` around lines 10 - 50, The package exports map contains five subpath entries (".", "./v3", "./v3/future", "./v4", "./v4/future") that currently use unconditional "types" pointing to .d.ts; update each of those entries to use the same conditional types mapping as in the conform-react Option A (provide both ESM and CJS declaration targets via conditional "types" / "import"/"require" keys) so both .d.ts and .d.cts variants are published, or instead choose Option B by disabling CJS dts generation in tsdown.config.mjs; apply the chosen fix consistently to the symbols ".", "./v3", "./v3/future", "./v4", and "./v4/future".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/conform-react/package.json`:
- Around line 26-29: The package.json files currently only include
"./dist/**/*.d.ts" which drops CJS declaration files (*.d.cts) emitted by
tsdown; update each packages/*/package.json (all six migrated packages) to
include "./dist/**/*.d.cts" in the "files" array and implement Option A: publish
both declaration variants and update the "exports" entries to provide
conditional paths so CJS consumers resolve to the .d.cts and ESM consumers to
.d.ts (adjust each export target for main/module/types to reference the matching
.d.cts/.d.ts), or alternatively disable CJS dts output in tsdown.config.mjs if
you prefer Option B—apply the chosen fix consistently across all six packages.
---
Duplicate comments:
In `@packages/conform-zod/package.json`:
- Around line 10-50: The package exports map contains five subpath entries (".",
"./v3", "./v3/future", "./v4", "./v4/future") that currently use unconditional
"types" pointing to .d.ts; update each of those entries to use the same
conditional types mapping as in the conform-react Option A (provide both ESM and
CJS declaration targets via conditional "types" / "import"/"require" keys) so
both .d.ts and .d.cts variants are published, or instead choose Option B by
disabling CJS dts generation in tsdown.config.mjs; apply the chosen fix
consistently to the symbols ".", "./v3", "./v3/future", "./v4", and
"./v4/future".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a44e81a6-2c0e-4a57-aa98-d8a4d4d10050
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
package.jsonpackages/conform-dom/package.jsonpackages/conform-dom/rollup.config.jspackages/conform-dom/tsconfig.build.jsonpackages/conform-dom/tsdown.config.mjspackages/conform-react/package.jsonpackages/conform-react/rollup.config.jspackages/conform-react/tsconfig.build.jsonpackages/conform-react/tsdown.config.mjspackages/conform-valibot/package.jsonpackages/conform-valibot/rollup.config.jspackages/conform-valibot/tsconfig.build.jsonpackages/conform-valibot/tsdown.config.mjspackages/conform-validitystate/package.jsonpackages/conform-validitystate/rollup.config.jspackages/conform-validitystate/tsconfig.build.jsonpackages/conform-validitystate/tsdown.config.mjspackages/conform-yup/package.jsonpackages/conform-yup/rollup.config.jspackages/conform-yup/tsconfig.build.jsonpackages/conform-yup/tsdown.config.mjspackages/conform-zod/package.jsonpackages/conform-zod/rollup.config.jspackages/conform-zod/tsconfig.build.jsonpackages/conform-zod/tsdown.config.mjs
💤 Files with no reviewable changes (12)
- packages/conform-zod/tsconfig.build.json
- packages/conform-react/tsconfig.build.json
- packages/conform-validitystate/tsconfig.build.json
- packages/conform-dom/tsconfig.build.json
- packages/conform-yup/tsconfig.build.json
- packages/conform-valibot/rollup.config.js
- packages/conform-yup/rollup.config.js
- packages/conform-dom/rollup.config.js
- packages/conform-valibot/tsconfig.build.json
- packages/conform-validitystate/rollup.config.js
- packages/conform-react/rollup.config.js
- packages/conform-zod/rollup.config.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/conform-zod/tsconfig.json`:
- Around line 15-16: Update the tsconfig.json "exclude" array to use a
consistent glob pattern style by changing the "dist" entry to "**/dist" so it
matches the existing "**/tests" pattern; modify the "exclude" field that
currently contains ["**/tests", "dist"] to ["**/tests", "**/dist"] to
future-proof nested build output directories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 270531a3-37bb-4bbf-a29c-42cefd803899
📒 Files selected for processing (7)
packages/conform-dom/tsconfig.jsonpackages/conform-react/tsconfig.jsonpackages/conform-react/tsdown.config.mjspackages/conform-valibot/tsconfig.jsonpackages/conform-validitystate/tsconfig.jsonpackages/conform-yup/tsconfig.jsonpackages/conform-zod/tsconfig.json
| }, | ||
| "exclude": ["**/tests", "dist"] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Consider pattern consistency for future-proofing.
The addition of the exclude field is appropriate for excluding test files and the build output directory from TypeScript's compilation scope, which aligns well with the migration to tsdown.
Minor optional suggestion: The pattern "**/tests" uses a glob to match at any depth, while "dist" only matches at the root level. For consistency and future-proofing, consider using "**/dist" to match the style of the tests pattern.
♻️ Optional refactor for pattern consistency
},
- "exclude": ["**/tests", "dist"]
+ "exclude": ["**/tests", "**/dist"]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, | |
| "exclude": ["**/tests", "dist"] | |
| }, | |
| "exclude": ["**/tests", "**/dist"] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/conform-zod/tsconfig.json` around lines 15 - 16, Update the
tsconfig.json "exclude" array to use a consistent glob pattern style by changing
the "dist" entry to "**/dist" so it matches the existing "**/tests" pattern;
modify the "exclude" field that currently contains ["**/tests", "dist"] to
["**/tests", "**/dist"] to future-proof nested build output directories.
1 participant
Generated Summary
This PR migrates the build toolchain of six packages (conform-dom, conform-react, conform-valibot, conform-validitystate, conform-yup, conform-zod) from Rollup + separate tsc steps to tsdown.
Net effect: consolidates and simplifies package builds to a single tsdown configuration per package while preserving ESM/CJS outputs and declaration generation.