chore: deps + security + drop EOL Node + tsup→Rollup + tsconfig cleanup#14
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:
📝 WalkthroughWalkthroughDependency and toolchain version bumps across root and app packages; Biome include pattern narrowed; TypeScript base and presets simplified/removed; apps/example tsconfig and deps updated; packages/leadtype migrated from tsup to Rollup with a new rollup.config.ts and tsup.config.ts removed. ChangesMonorepo-Wide Dependency Updates & Build Migration
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Rollup
participant Esbuild
participant DTS
Dev->>Rollup: run rollup -c
Rollup->>Esbuild: bundle ESM entries (es2022 target)
Rollup->>DTS: generate .d.ts files
Rollup->>Rollup: add CLI shebang for cli chunk
Rollup->>Rollup: set dist/cli.js executable (chmod)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 517c828018
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "unist-util-is": "6.0.1", | ||
| "unist-util-visit": "5.1.0", | ||
| "valibot": "1.0.0" | ||
| "valibot": "1.4.0" |
There was a problem hiding this comment.
Because leadtype is a published package and the release workflow I inspected uses changesets/action with bun run version/bun run release, this dependency change will not produce a new package version unless the commit also adds a .changeset entry. In that case the valibot advisory fix stays only in the repo/lockfile and consumers installing leadtype from npm will continue receiving the last published package with valibot 1.0.0.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/example/package.json`:
- Around line 62-63: Update the root package.json "engines.node" minimum to
match Vite 8's requirement by setting the Node engine floor to at least 20.19
(e.g., ">=20.19.0") so it is consistent with apps/example using Vite v8; locate
and edit the "engines" object in the root package.json (the property currently
set to "node": ">=18") and raise the version constraint accordingly, then run
CI/lockfile tasks to ensure all toolchains and consumers see the updated Node
requirement.
🪄 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: 45e99cb3-6f9b-4b89-a011-a1a2df1eb40f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/example/package.jsonbiome.jsoncpackage.jsonpackages/leadtype/package.json
💤 Files with no reviewable changes (1)
- biome.jsonc
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
🔍 Remote MCP Context7
Summary of Additional Context for PR Review
Based on my research, here are the key findings relevant to this dependency update PR:
Critical Security Fixes
Valibot 1.4.0 – ReDoS Vulnerability (High)
The v1.2+ releases address a ReDoS (Regular Expression Denial of Service) vulnerability in the EMOJI_REGEX pattern. This vulnerability could allow attackers to trigger excessive CPU usage with specially crafted input strings. The team has updated the regex pattern while maintaining validation functionality.
Nitro Security Fixes (Moderate)
The Nitro upgrade addresses route rule security concerns documented in their routing configuration system. The library provides route rules for managing wildcards, redirects, and proxies. Organizations should verify that any existing wildcard route configurations (particularly those using '**' patterns) remain secure after the upgrade.
Major Version Migrations to Verify
Vite 7 → 8
According to Vite documentation, the upgrade includes a built-in compatibility layer that auto-converts existing esbuild and rollupOptions configurations. The migration is intended to be smooth for most projects. Vite provides a future configuration option to opt-in to breaking changes in advance.
Vitest 2 → 4
The PR shows 97/97 tests passing, which is excellent. However, be aware of these breaking changes in Vitest 4.0:
- Test options argument reordering: Options moved to second argument position (previously third)
- Deprecated APIs removed:
poolMatchGlobsandenvironmentMatchGlobsreplaced byprojectsconfiguration - Exclude patterns changed: Default exclusions for
distandcypressdirectories were removed; you may need to manually restore them if relied upon
@types/node 22 → 25
A significant jump in Node type definitions. Review any type-dependent code to ensure compatibility.
Biome Configuration Change
The removal of "**" from files.includes in biome.jsonc aligns with Biome 2.4.14's behavior when extending an ultracite configuration. Biome's file configuration documentation shows files.include and files.ignore patterns control which files are processed. This narrower pattern scope should be validated to ensure all necessary files are still being linted/formatted.
Testing Coverage
✅ The PR demonstrates comprehensive testing:
- 97/97 Vitest tests passing (vitest 4)
- All builds successful (vite 8)
- No type errors across workspaces
- Clean dependency installation (1590 packages)
🔇 Additional comments (4)
package.json (1)
3-13: Dependency/tooling bumps look consistent with the PR objective.No issues in this segment; version updates are coherent and scoped.
apps/example/package.json (1)
35-40: Security-driven dependency updates are good here.The
nitrobump and related dependency updates align with the advisory remediation intent.packages/leadtype/package.json (2)
94-94: Valibot security bump is correctly applied.This update aligns with the stated advisory remediation.
99-103: Peer minimums are aligned with the upgraded dependency set.Good to see
@tanstack/aiandjust-bashpeer constraints updated alongside the dependency bump.Also applies to: 112-115
KayleeWilliams
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/rollup.config.ts`:
- Around line 6-20: Make the entries object literal explicitly immutable by
adding a const assertion (use "entries" with as const) so the keys and string
values are typed as literals, and replace the inline file mode magic number
0o755 with a clearly named constant (e.g., EXECUTABLE_MODE or FILE_MODE_EXECUTE)
declared near the top of the module and used where 0o755 currently appears;
update references in rollup.config.ts to use the new constant and keep entries
as a const-assigned variable (entries as const) to satisfy stronger literal
typing.
🪄 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: f31f409b-3165-4f88-a127-324babadd3e3
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
package.jsonpackages/leadtype/package.jsonpackages/leadtype/rollup.config.tspackages/leadtype/tsup.config.ts
💤 Files with no reviewable changes (1)
- packages/leadtype/tsup.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/rollup.config.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/rollup.config.ts
🪛 GitHub Actions: Benchmark / 0_Convert + LLM pipeline.txt
package.json
[error] 1-1: Step failed: script "build" exited with code 1
packages/leadtype/rollup.config.ts
[error] 1-1: Rollup build failed: TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /home/runner/work/leadtype/leadtype/packages/leadtype/rollup.config.ts
packages/leadtype/package.json
[error] 1-1: Step failed: script "build" exited with code 1
🪛 GitHub Actions: Benchmark / Convert + LLM pipeline
packages/leadtype/rollup.config.ts
[error] 1-1: Build failed: TypeError: Unknown file extension ".ts" for /home/runner/work/leadtype/leadtype/packages/leadtype/rollup.config.ts (ERR_UNKNOWN_FILE_EXTENSION). Script "build" exited with code 1.
[error] 1-1: Command failed: "cd packages/leadtype && bun run build" (runs "rollup -c && bun run docs:generate").
packages/leadtype/package.json
[error] 1-1: Command failed: "cd packages/leadtype && bun run build" (runs "rollup -c && bun run docs:generate").
🔍 Remote MCP Context7
Summary of additional relevant facts for reviewing PR #14
-
Valibot — ReDoS in EMOJI_REGEX
- Valibot v1.2 release notes state a critical ReDoS (regular-expression DoS) in the EMOJI_REGEX used by the emoji validator was fixed; users are urged to upgrade (v1.2+ addresses it). The PR’s bump to 1.4.0 therefore includes that fix.,
-
Nitro — routeRules redirect/proxy wildcard risks
- Nitro docs for routeRules show wildcard redirects/proxies (e.g., '/old-blog/' -> 'https://blog.example.com/') and proxy examples; the cited Nitro security fixes (open redirect via protocol-relative URLs and percent-encoded path-traversal proxy bypass) are directly relevant to any wildcard redirect/proxy rules in apps/example. Verify the app’s routeRules/wildcard patterns for unintended open-redirect or proxy targets after the bump.,
-
Vite 7 → 8 migration notes
- Vite 8 introduces Rolldown/Oxc replacements and provides a compatibility layer to auto-convert many esbuild/rollupOptions cases; migration guidance recommends a gradual path (rolldown-vite on v7 → v8) for complex projects. The
futureconfig can opt into/warn on upcoming breaking changes. Confirm custom rollup/esbuild options or plugin behaviors (e.g., rollup-plugin-esbuild usage in new rollup.config.ts) behave as expected under Vite 8.,
- Vite 8 introduces Rolldown/Oxc replacements and provides a compatibility layer to auto-convert many esbuild/rollupOptions cases; migration guidance recommends a gradual path (rolldown-vite on v7 → v8) for complex projects. The
Tools/queries used
- Resolved libraries: Valibot (/open-circle/valibot), Nitro (/unjs/nitro), Vite (/vitejs/vite).
- Documentation/changelog queries for ReDoS fix (Valibot), routeRules/redirects/proxy examples (Nitro), and Vite 8 migration/compatibility notes (Vite).
🔇 Additional comments (3)
package.json (1)
3-13: Dependency/tooling and engine updates look aligned with PR intent.No blocker found in this file’s changed lines.
Also applies to: 37-37
packages/leadtype/package.json (2)
97-113: Security/tooling dependency bumps here are directionally correct.The
valibotand build-toolchain version updates align with the PR security and modernization goals.
71-72: ⚡ Quick winBuild may need config transpilation for rollup.config.ts, but verify the correct plugin.
rollup 4.40.0 requires a
--configPluginoption to transpile TypeScript config files. The current build and dev scripts lack this. However, the proposed fix specifiesesbuild, while rollup's TypeScript support typically usestypescriptas the config plugin. Sincerollup-plugin-typescriptis not installed (onlyrollup-plugin-esbuildis), either install the TypeScript plugin or confirm that esbuild can serve as a valid config plugin for this use case. Additionally, the CI currently runs the build successfully, so verify whether this is an actual blocker or a potential issue.
| const entries = { | ||
| index: "src/index.ts", | ||
| "remark/index": "src/remark/index.ts", | ||
| "convert/index": "src/convert/index.ts", | ||
| "llm/index": "src/llm/index.ts", | ||
| "search/index": "src/search/index.ts", | ||
| "search/node-index": "src/search/node-index.ts", | ||
| "search/ai-index": "src/search/ai-index.ts", | ||
| "search/bash-index": "src/search/bash-index.ts", | ||
| "search/vercel-index": "src/search/vercel-index.ts", | ||
| "search/tanstack-index": "src/search/tanstack-index.ts", | ||
| "search/cloudflare-index": "src/search/cloudflare-index.ts", | ||
| "lint/index": "src/lint/index.ts", | ||
| cli: "src/cli.ts", | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Make immutable config literals explicit and replace inline mode magic number.
Line 6-20 can use as const for stronger literal typing, and Line 32 should use a named constant for 0o755.
Proposed refactor
+const CLI_EXECUTABLE_MODE = 0o755;
+
const entries = {
index: "src/index.ts",
"remark/index": "src/remark/index.ts",
"convert/index": "src/convert/index.ts",
"llm/index": "src/llm/index.ts",
"search/index": "src/search/index.ts",
"search/node-index": "src/search/node-index.ts",
"search/ai-index": "src/search/ai-index.ts",
"search/bash-index": "src/search/bash-index.ts",
"search/vercel-index": "src/search/vercel-index.ts",
"search/tanstack-index": "src/search/tanstack-index.ts",
"search/cloudflare-index": "src/search/cloudflare-index.ts",
"lint/index": "src/lint/index.ts",
cli: "src/cli.ts",
-};
+} as const;
@@
- await chmod(`${options.dir}/cli.js`, 0o755);
+ await chmod(`${options.dir}/cli.js`, CLI_EXECUTABLE_MODE);As per coding guidelines, "Use const assertions (as const) for immutable values and literal types" and "Use meaningful variable names instead of magic numbers - extract constants with descriptive names".
Also applies to: 32-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/rollup.config.ts` around lines 6 - 20, Make the entries
object literal explicitly immutable by adding a const assertion (use "entries"
with as const) so the keys and string values are typed as literals, and replace
the inline file mode magic number 0o755 with a clearly named constant (e.g.,
EXECUTABLE_MODE or FILE_MODE_EXECUTE) declared near the top of the module and
used where 0o755 currently appears; update references in rollup.config.ts to use
the new constant and keep entries as a const-assigned variable (entries as
const) to satisfy stronger literal typing.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/example/package.json`:
- Line 51: The dependency entry for "@repo/typescript-config" uses a wildcard
"*" which prevents reproducible installs; update the package.json dependency for
"@repo/typescript-config" to use "workspace:*" or a specific semver/version
(e.g., "1.2.3" or "^1.2.0") instead of "*" so the workspace dependency is pinned
and installs are deterministic.
In `@packages/leadtype/tsconfig.json`:
- Line 7: tsconfig.json declares "types": ["node"] but the project targets Node
>=22 while package devDependencies currently use `@types/node`@^25.6.2; update the
devDependency "@types/node" to a 22.x.x range (e.g. ^22.0.0) in package.json,
run your package manager to reinstall lockfiles, and ensure the "types" entry in
tsconfig.json remains "node" so TypeScript picks up the correct `@types/node`@22
definitions.
🪄 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: 3c9f4d36-0f58-4821-85ab-a183eba8d74d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/example/package.jsonapps/example/tsconfig.jsonpackages/leadtype/tsconfig.jsonpackages/typescript-config/base.jsonpackages/typescript-config/nextjs.jsonpackages/typescript-config/react-library.json
💤 Files with no reviewable changes (2)
- packages/typescript-config/nextjs.json
- packages/typescript-config/react-library.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
🔍 Remote MCP Context7
Summary of additional facts relevant to reviewing PR #14
-
Valibot: v1.2 release notes state a critical ReDoS in the EMOJI_REGEX (emoji validator) was fixed and users are strongly encouraged to upgrade; the PR’s bump to 1.4.0 therefore contains that remediation.
-
Nitro: docs show routeRules support wildcard redirects and proxy rules (e.g., '/old-blog/' -> 'https://blog.example.com/' and '/proxy/' -> '/api/'); the cited Nitro security fixes (open redirect via protocol-relative URLs and percent-encoded path-traversal proxy bypass) are directly relevant — reviewers should inspect apps/example routeRules/wildcard redirect or proxy rules for unsafe targets or patterns.
-
Vite 8: official migration guidance notes Vite 8 includes a compatibility layer that auto-converts many esbuild/rollupOptions cases and recommends a gradual path (rolldown-vite → Vite 8) for complex projects; reviewers should confirm custom rollup/esbuild options or plugins (and the new rollup usage in packages/leadtype) remain compatible after the Vite bump.
Tools/sources used
- Context7 resolver queries and docs for Valibot, Nitro, and Vite.,
🔇 Additional comments (9)
packages/typescript-config/base.json (1)
8-11: Config baseline looks consistent and modern.The
ES2022+ESNext+Bundler+verbatimModuleSyntaxcombination is coherent for a shared TS base and aligns with the PR goal of standardizing compiler behavior.Also applies to: 16-17
apps/example/tsconfig.json (1)
3-3: Shared tsconfig inheritance change looks good.Using the workspace config package path here is cleaner than a deep relative path and keeps configuration ownership centralized.
apps/example/package.json (7)
35-36: LGTM: TanStack Router updates.Minor version bumps within the v1 range are safe and maintain API compatibility.
45-45: LGTM: Tailwind v4.3 minor bump.The 4.2.1 → 4.3.0 update within Tailwind v4 should be safe. If this is the first v4 adoption, ensure the codebase has migrated breaking changes (important modifier syntax
text-xl!, opacity slash syntaxbg-blue-500/20, etc.).Also applies to: 52-52
59-59: ⚡ Quick win@vitejs/plugin-react v6 is compatible with React 19.
The plugin was released specifically to support React 19 and includes explicit handling for React 19's compiler behavior and runtime optimization. No compatibility verification needed.
56-56: ⚡ Quick winRemove concern about
@types/nodev25.6.2 validity. The version is a legitimate release published on npm (May 7, 2026) and is available for installation. The major version bump from v22 to v25 reflects the actual progression of the@types/nodepackage.
53-53: ⚖️ Poor tradeoffTypeScript 6.0.3 is a stable release, not experimental.
The concern about
typescript@6.0.3being a pre-release version is incorrect—it is a standard stable release. However,@typescript/native-preview@7.0.0-dev.20260509.2(line 53) is genuinely a dev preview version with nightly-based versioning, which may warrant a separate evaluation of production readiness.> Likely an incorrect or invalid review comment.
40-40: ⚡ Quick winExact pinning of nitro beta version prevents automatic patch updates.
The version
3.0.260429-betahas no caret, meaning no automatic patch updates will be applied. For beta versions that receive frequent updates, consider whether this strict pinning is intentional or if a wildcard pattern (3.0.260429-*) would better balance stability with critical patches.> Likely an incorrect or invalid review comment.
63-64: ⚡ Quick winNo compatibility concerns with Vite 8 and vite-tsconfig-paths v6 in this configuration.
The Vite config uses standard plugins and no custom esbuild/rollup options that would be affected by Vite 8's toolchain change. vite-tsconfig-paths v6 has no breaking changes and is confirmed as a drop-in replacement by maintainers. Both versions are compatible with the current setup.
- valibot 1.0.0 → 1.4.0 (CVE: ReDoS in EMOJI_REGEX) - nitro 3.0.260415-beta → 3.0.260429-beta (CVE: open redirect via protocol-relative URL bypass + proxy scope bypass via percent-encoded path traversal in routeRules) - vite 7 → 8, vitest 2 → 4, @vitejs/plugin-react 5 → 6, vite-tsconfig-paths 5 → 6, @types/node 22 → 25, @changesets/changelog-github 0.6 → 0.7 - minor/patch bumps across @biomejs/biome, @changesets/cli, turbo, ultracite, @typescript/native-preview, @tanstack/react-router, @tanstack/react-start, ai, tailwindcss, @tanstack/ai, just-bash - biome.jsonc: drop "**" from files.includes (biome 2.4.14 forbids catch-all when extending an ultracite config that already has one)
- engines: bump root + leadtype from >=18 to >=22.0.0. Node 18 EOL, Node 20 EOL April 2026; supported Active LTS floor is 22. - leadtype: replace tsup with Rollup 4 + rollup-plugin-esbuild + rollup-plugin-dts. ESM-only output preserved (already no CJS in source or exports map). 13 entry points + cli bin keep their paths so the package.json exports map is unchanged. - rollup.config.ts: cleanDist plugin clears dist before the JS pass (Rollup has no built-in clean), banner injects shebang only on the cli chunk, chmodCli sets 0755 in writeBundle. - Removes the artificial per-entry duplication tsup imposed via splitting:false; shared modules now live under dist/_shared/ with content-hashed names.
base.json now reflects what every consumer actually wants: - lib: ["ES2022"] (DOM moves down to apps/example, the only consumer that needs it). Stops leadtype source from accidentally referencing document/window/etc. - module/moduleResolution: ESNext + Bundler. Every consumer was already overriding NodeNext to this; the base was a fiction. - verbatimModuleSyntax: true. ultracite already enforces useImportType, so this is a redundancy gate rather than a new constraint. - Drop incremental:false (no-op default). leadtype/tsconfig.json: drop module/moduleResolution overrides (now in base) and stop weakening noUncheckedIndexedAccess + verbatimModuleSyntax. The publishable library shouldn't be the least strict config in the repo. Source was already index-safe via existing optional chaining and length guards — zero src/ changes needed. apps/example/tsconfig.json: drop redundant target/module/moduleResolution /verbatimModuleSyntax (now inherited). Switch extends from relative path to package alias and declare @repo/typescript-config in package.json devDeps to match. Delete unreferenced typescript-config/nextjs.json and react-library.json (zero consumers across the repo).
- @repo/typescript-config: "*" → "workspace:*" in apps/example and packages/leadtype. Matches the existing "leadtype": "workspace:*" pattern; explicit-better-than-implicit, and removes the chance of a non-Bun tool trying to resolve "*" against the npm registry. - packages/leadtype/@types/node: ^25.6.2 → ^22.0.0. The library declares engines.node >=22.0.0, so its types should match the lowest supported runtime — otherwise consumers on Node 22 LTS could pick up types for Node 25-only APIs that don't exist at runtime. Resolves to @types/node@22.19.17 in the workspace. - apps/example/@types/node stays at ^25.6.2 deliberately: the app is private (never published) and its devs run whatever Node they have locally, so the latest-types choice is fine there.
KayleeWilliams
force-pushed
the
KayleeWilliams/Nautilus
branch
from
fa49c0e to
c514084
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/leadtype/rollup.config.ts (2)
6-20: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse const assertion for immutable entries map.
The
entriesobject is a static configuration that should be explicitly immutable to leverage literal type inference.♻️ Proposed fix
const entries = { index: "src/index.ts", "remark/index": "src/remark/index.ts", "convert/index": "src/convert/index.ts", "llm/index": "src/llm/index.ts", "search/index": "src/search/index.ts", "search/node-index": "src/search/node-index.ts", "search/ai-index": "src/search/ai-index.ts", "search/bash-index": "src/search/bash-index.ts", "search/vercel-index": "src/search/vercel-index.ts", "search/tanstack-index": "src/search/tanstack-index.ts", "search/cloudflare-index": "src/search/cloudflare-index.ts", "lint/index": "src/lint/index.ts", cli: "src/cli.ts", -}; +} as const;As per coding guidelines, "Use const assertions (
as const) for immutable values and literal types".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/leadtype/rollup.config.ts` around lines 6 - 20, The entries configuration object is intended to be immutable but lacks a const assertion; update the declaration of the entries variable (symbol: entries in rollup.config.ts) to use a const assertion (e.g., append "as const" to the object) so TypeScript preserves literal types and prevents accidental mutation, ensuring downstream code benefits from exact key/value literal types.
32-32: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winExtract file mode magic number into named constant.
The octal literal
0o755should be a named constant for clarity and maintainability.♻️ Proposed fix
+const CLI_EXECUTABLE_MODE = 0o755; + const entries = {- await chmod(`${options.dir}/cli.js`, 0o755); + await chmod(`${options.dir}/cli.js`, CLI_EXECUTABLE_MODE);As per coding guidelines, "Use meaningful variable names instead of magic numbers - extract constants with descriptive names".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/leadtype/rollup.config.ts` at line 32, The chmod call uses a magic octal literal (0o755); extract this into a descriptive constant (e.g., CLI_EXECUTABLE_MODE or FILE_MODE_CLI_EXECUTABLE) and replace the literal in the chmod invocation (the line calling chmod(`${options.dir}/cli.js`, 0o755)) with that constant; declare the constant near the top of the module (exported or file-scoped as appropriate) so the permission meaning is clear and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/package.json`:
- Around line 72-73: The "build" and "dev" npm scripts currently invoke "rollup
-c" which uses Node's Rollup CLI and fails to load TypeScript configs; update
the scripts named "build" and "dev" to prefix the rollup command with "bun"
(i.e., use "bun rollup -c" and "bun rollup -c -w") so Bun's runner and
TypeScript support are used.
In `@packages/leadtype/rollup.config.ts`:
- Around line 1-69: The Rollup config file rollup.config.ts cannot be loaded by
Node's ESM loader when running the existing build script that invokes "rollup
-c" because TypeScript files require Rollup's config plugin; update your build
command to pass the TypeScript config plugin (e.g. change the script invoking
rollup -c to use --config rollup.config.ts --configPlugin typescript) so the CLI
can load rollup.config.ts (the file that defines entries, isExternal,
cliShebang, chmodCli, cleanDist, and the default defineConfig export);
alternatively you can switch to bun rollup -c or convert rollup.config.ts to a
JS config, but prefer adding --configPlugin typescript per Rollup docs.
---
Duplicate comments:
In `@packages/leadtype/rollup.config.ts`:
- Around line 6-20: The entries configuration object is intended to be immutable
but lacks a const assertion; update the declaration of the entries variable
(symbol: entries in rollup.config.ts) to use a const assertion (e.g., append "as
const" to the object) so TypeScript preserves literal types and prevents
accidental mutation, ensuring downstream code benefits from exact key/value
literal types.
- Line 32: The chmod call uses a magic octal literal (0o755); extract this into
a descriptive constant (e.g., CLI_EXECUTABLE_MODE or FILE_MODE_CLI_EXECUTABLE)
and replace the literal in the chmod invocation (the line calling
chmod(`${options.dir}/cli.js`, 0o755)) with that constant; declare the constant
near the top of the module (exported or file-scoped as appropriate) so the
permission meaning is clear and maintainable.
🪄 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: 83c231c3-c482-4ec4-82b0-4e458ba6c7ab
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
apps/example/package.jsonapps/example/tsconfig.jsonbiome.jsoncpackage.jsonpackages/leadtype/package.jsonpackages/leadtype/rollup.config.tspackages/leadtype/tsconfig.jsonpackages/leadtype/tsup.config.tspackages/typescript-config/base.jsonpackages/typescript-config/nextjs.jsonpackages/typescript-config/react-library.json
💤 Files with no reviewable changes (4)
- biome.jsonc
- packages/typescript-config/react-library.json
- packages/leadtype/tsup.config.ts
- packages/typescript-config/nextjs.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/rollup.config.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/rollup.config.ts
🪛 GitHub Actions: Benchmark / 0_Convert + LLM pipeline.txt
packages/leadtype/package.json
[error] 1-1: Command failed: "bun run build" (includes "rollup -c && bun run docs:generate"). Script "build" exited with code 1.
packages/leadtype/rollup.config.ts
[error] 1-1: Build failed while running Rollup config. TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /home/runner/work/leadtype/leadtype/packages/leadtype/rollup.config.ts
[error] 1-1: Command failed: "bun run build" (includes "rollup -c && bun run docs:generate"). Script "build" exited with code 1.
packages/leadtype/tsconfig.json
[error] 1-1: Command failed: "bun run build" (includes "rollup -c && bun run docs:generate"). Script "build" exited with code 1.
🪛 GitHub Actions: Benchmark / Convert + LLM pipeline
packages/leadtype/rollup.config.ts
[error] 1-1: Rollup build failed with TypeError: Unknown file extension ".ts" for rollup.config.ts. Node ESM cannot load the TypeScript rollup config (ERR_UNKNOWN_FILE_EXTENSION).
🔍 Remote MCP Context7
Summary of additional facts relevant to reviewing PR #14
-
Valibot: v1.2 fixed a critical ReDoS in the EMOJI_REGEX used by the emoji action; maintainers strongly recommend upgrading (relevant to this PR’s bump to 1.4.0). Inspect any usage of valibot’s emoji action or regex-based validators in the repo.
-
Nitro (unjs/nitro): Nitro routeRules support redirects and proxying with wildcard patterns (e.g., '/old-blog/' → 'https://blog.example.com/' and '/proxy/' → '/api/'). Because the PR references Nitro security fixes (open redirect via protocol-relative targets and percent-encoded path-traversal proxy bypass), review any nitro.routeRules, redirects, or proxy targets for unvalidated inputs, protocol-relative targets, or percent-encoded path handling.
-
Rollup: Rollup outputs support per-output banner/footer (useful for injecting a CLI shebang) and plugins/hooks (renderChunk, generateBundle, writeBundle) can modify emitted code and file permissions. Typical approaches to adding a shebang and setting executable permissions are: add a banner for the CLI chunk and use writeBundle/writeFile hooks to set mode (chmod) or emit assets. Confirm the new rollup config’s shebang/banner + chmodCli implementation uses writeBundle/writeFile hooks safely and preserves expected output names/exports.
Files/areas to inspect in this PR with these facts in mind
- Any code using valibot's emoji validator or custom regex validators (search for v.emoji / EMOJI_REGEX).
- nitro config files (nitro.config.ts or similar) and any routeRules/redirect/proxy definitions.
- packages/leadtype/rollup.config.ts:
- verify the CLI chunk receives a proper banner shebang and that the chmodCli step sets 0o755 reliably in writeBundle/writeFile.
- verify externals/preserved exports and dist layout match previous tsup outputs.
- Type/engine alignment:
- engines.node bumps to >=22.0.0 — ensure
@types/nodeand any native-target builds align.
- engines.node bumps to >=22.0.0 — ensure
Tools/sources used
- Valibot release notes / docs (Context7 query for valibot ReDoS and EMOJI_REGEX)
- Nitro routing/proxy/redirect docs (Context7 query for nitro routeRules and proxy examples)
- Rollup docs (banner/output options and plugin hooks for bundle generation / writeBundle for applying shebang/banner and setting permissions)
🔇 Additional comments (14)
package.json (2)
37-37: LGTM: Node floor raised to match Vite 8 and package requirements.The
engines.nodebump to>=22.0.0aligns withpackages/leadtype/package.jsonand satisfies Vite 8's requirement (20.19+). This ensures consistency across the monorepo.
3-13: LGTM: Dependency updates address security advisories.The dependency bumps include security-critical updates (valibot for ReDoS fix noted in external context) and align with the PR's security objectives.
apps/example/package.json (2)
52-52: LGTM: Workspace dependency properly pinned.The
workspace:*protocol ensures deterministic resolution for the monorepo dependency.
41-41: LGTM: Nitro version updated to address security advisories.This version (3.0.260429-beta) includes fixes for the open redirect and proxy scope bypass vulnerabilities mentioned in the PR objectives and external context.
packages/typescript-config/base.json (2)
8-11: LGTM: Base config tuned for ESM + Bundler workflow.The removal of DOM libs and switch to
moduleResolution: "Bundler"align with the Rollup migration and ESM-only output strategy. Apps/example properly overrideslibto add DOM where needed.
17-17: LGTM:verbatimModuleSyntaxenforces explicit imports.This stricter mode prevents ambiguous import/export syntax and aligns with modern TypeScript best practices.
apps/example/tsconfig.json (1)
3-3: LGTM: Package-path extension simplifies config management.Switching from a relative path to
@repo/typescript-config/base.jsonmakes the config more portable and aligns with the addeddevDependency.packages/leadtype/tsconfig.json (1)
7-7: LGTM: Minimal config delegates to shared base.The simplified
compilerOptionsproperly inherits module/resolution settings from the base config and restricts to Node types.packages/leadtype/rollup.config.ts (2)
22-23: LGTM: Clean external dependency detection.The
isExternalpredicate correctly identifies Node built-ins and npm packages while preserving relative imports for bundling.
28-35: LGTM: CLI permissions correctly applied in writeBundle.The
chmodCliplugin properly uses thewriteBundlehook to set executable permissions after the bundle is written, which is the correct Rollup lifecycle hook for this operation.packages/leadtype/package.json (4)
10-12: LGTM: Node engine floor set to 22.This aligns with the root
package.jsonand ensures consistent runtime requirements across the monorepo.
98-98: LGTM: Valibot upgrade addresses ReDoS vulnerability.The bump from 1.0.0 to 1.4.0 includes the critical fix for the EMOJI_REGEX ReDoS vulnerability (High severity) mentioned in the external context and Dependabot advisories.
106-106: LGTM:@types/nodeversion correctly aligned with runtime.Using
@types/node@^22.0.0properly matchesengines.node: ">=22.0.0", following the@types/nodeversioning convention where the major version must match the Node.js major version.
110-112: LGTM: Rollup tooling properly configured.The combination of
rollup,rollup-plugin-dts, androllup-plugin-esbuildreplaces the previoustsupsetup and supports the new build configuration inrollup.config.ts.
Documents the entries map as static config data rather than mutable state. No runtime change — Rollup's input type accepts the readonly form. Cheap, but skipped the reviewer's other rollup.config suggestions: - "rollup -c" → "bun rollup -c": already works. Rollup 4 natively loads rollup.config.ts when typescript is installed (verified across every build this session). - --configPlugin typescript: same — unnecessary on Rollup 4. - 0o755 → named constant: 0o755 is itself a Unix idiom for rwxr-xr-x; used once; extracting adds indirection without clarity.
CI runs rollup -c under Node, which can't load rollup.config.ts: TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" Locally this masquerades as working because Bun's runtime intercepts rollup's CLI shebang and handles .ts files natively. On a stock Node runner that doesn't happen. Reversing my earlier triage — finding 1 in the last review WAS valid; I missed it because my local environment hid the bug. Trust CI.
bun rollup -c didn't help: bun resolved the rollup binary but the spawned node process couldn't load .ts files. Switch to Rollup's documented flow for TS configs — pass --configPlugin so Rollup uses the named plugin to transform its own config before loading. Routing through rollup-plugin-esbuild (already a dep) avoids any new deps.
1 participant
Summary
Four coupled cleanups before the leadtype library publishes.
1. Dependencies + security advisories
Closes the three open Dependabot advisories (1 high, 2 moderate):
valibot1.0.0 → 1.4.0 — High: ReDoS inEMOJI_REGEX(packages/leadtype)nitro3.0.260415-beta → 3.0.260429-beta — Moderate: open redirect via protocol-relative URL bypass (apps/example)nitro(same bump) — Moderate: proxy scope bypass via percent-encoded path traversal inrouteRulesMajor bumps (verified):
vite7→8,vitest2→4 (97/97 passing),@vitejs/plugin-react5→6,vite-tsconfig-paths5→6,@types/node22→25,@changesets/changelog-github0.6→0.7.Minor/patch bumps:
@biomejs/biome2.4.14,@changesets/cli2.31.0,turbo2.9.12,ultracite7.6.5,@typescript/native-preview20260509.2,@tanstack/react-router1.169.2,@tanstack/react-start1.167.65,ai6.0.177,tailwindcss4.3.0,@tanstack/ai0.15.0,just-bash2.14.5.Side-effect fix: Biome 2.4.14 added a rule against
**infiles.includeswhen extending a config with its own catch-all. Removed frombiome.jsonc:15.2. Drop EOL Node, raise floor to 22
engines.node:>=18→>=22.0.0(root +packages/leadtype).3. tsup → Rollup migration for
packages/leadtypeReplaces
tsup@8.xwithrollup@4+rollup-plugin-esbuild+rollup-plugin-dts.require()/__dirname/module.exports.package.jsonhas nomainand norequirekeys inexports."type": "module"set. tsup was alreadyformat: ["esm"].clibin write to the samedist/paths the existingexportsmap references.output.bannerfires only on theclichunk; smallchmodCliplugin sets0755inwriteBundle.cleanDistplugin clearsdist/once at the start of the JS pass (not the dts pass — that would wipe the output we just produced). Rollup has no built-in clean.dist/_shared/[name]-[hash].js. tsup hadsplitting: falsewhich inflated per-entry bundles.4. tsconfig overhaul
base.jsonis now the actual baseline every consumer uses:lib: ["ES2022"]— DOM dropped from base, pushed toapps/example(the only consumer that needs it). Stopsleadtypesource from accidentally referencingdocument/window/etc.module: ESNext+moduleResolution: Bundler— matches what every consumer was overriding to. Reflects Rollup/Vite reality.verbatimModuleSyntax: true— ultracite already enforcesuseImportType, so this is a redundancy gate.incremental: falseremoved (no-op default).packages/leadtype/tsconfig.json:module/moduleResolutionoverrides (now inherited from base).noUncheckedIndexedAccess: falseandverbatimModuleSyntax: false— the publishable library is no longer the least-strict config in the repo. Source was already index-safe via existing?.and length guards; zerosrc/changes needed.apps/example/tsconfig.json:target/module/moduleResolution/verbatimModuleSyntax(all inherited).extendsfrom relative path to package alias.@repo/typescript-config: "*"to devDeps (was missing).Deleted dead
typescript-config/nextjs.jsonandreact-library.json— zero consumers anywhere in the repo.Test plan
bun installcleanbun run check-types(turbo) — clean across all 3 workspaces with the strictness flags now flipped onbun run check(ultracite/biome 2.4.14) — cleanbun run test— vitest 4, 97/97 passing inpackages/leadtypebun run build(turbo) — example builds on Vite 8, leadtype builds on Rollup 4dist/cli.jskeeps#!/usr/bin/env nodeshebang +0755modedist/layout matches the unchangedexportsmap (e.g.dist/search/cloudflare-index.js)🤖 Generated with Claude Code