perf(core): lazy-load config deps & treeshake barrel exports#123
perf(core): lazy-load config deps & treeshake barrel exports#123zrosenbauer merged 10 commits intomainfrom
Conversation
…in fp barrel The `export * from 'es-toolkit'` and `export * from 'ts-pattern'` in the fp barrel module prevented tree-shaking in downstream CLI bundles. Only 7 es-toolkit functions are used across the codebase, but all 193 modules were bundled. Narrows exports to the specific functions consumed: attempt, attemptAsync, isFunction, isNil, isPlainObject, isString, noop, match, P. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
🦋 Changeset detectedLatest commit: 5a19f17 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will create unknown performance changes🎉 Hooray!
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| 🆕 | WallTime | icons status |
N/A | 411.9 ms | N/A |
| 🆕 | WallTime | icons --help |
N/A | 392.6 ms | N/A |
Comparing perf/treeshake-barrel-exports (5a19f17) with main (9a6fa77)
Footnotes
-
2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
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:
📝 WalkthroughWalkthroughReplaced a wildcard export from Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Dynamic import() the config loader (c12 → yaml, jiti, confbox) only when actually needed — not during yargs setup/help rendering. This cuts ~15K lines from the startup parse path for --help and --version. Co-Authored-By: Claude <noreply@anthropic.com>
es-toolkit declares "sideEffects": false, so export * tree-shakes correctly. Explicit named exports just add maintenance burden. Co-Authored-By: Claude <noreply@anthropic.com>
Add createInProcessRunner that dynamically imports the built CLI entry with process.argv/exit/stdout/stderr stubs. This runs benchmarks in the same V8 process so CodSpeed can resolve interpreted frame symbols and generate flamegraphs. Switch icons benchmark from subprocess (createExampleRunner) to in-process (createInProcessRunner) to fix "Execution profile not available" in CodSpeed. Co-Authored-By: Claude <noreply@anthropic.com>
Keep updateSettings from main, keep loadConfig lazy-loaded from our branch. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/helpers.ts`:
- Around line 98-99: The code builds entryPath (const entryPath =
`${EXAMPLES_DIR}/${example}/${distPath}`) and then passes it directly to
import(), which breaks on Windows; instead convert the filesystem path to a
file:// URL using pathToFileURL(entryPath) and add the cache-bust query via the
returned URL's searchParams before calling import(); update usages around
entryPath and the dynamic import to call pathToFileURL and use
url.searchParams.append(...) so import(url.toString()) receives a proper file://
URL.
- Around line 100-141: The createInProcessRunner function mutates process
globals (process.argv/process.exit/process.stdout.write/process.stderr.write)
making it re-entrancy unsafe; wrap the section that applies and restores these
global patches and imports entryPath in a single-flight/async-mutex so only one
runner can monkeypatch at a time, await the mutex before applying the patches,
release it in the finally block after restoring originals, and ensure the lock
is held across the await import(`${entryPath}?t=${Date.now()}`) to prevent
interleaving between concurrent invocations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f418f2e-62a5-43ac-8ca4-cd9677cbfe0d
📒 Files selected for processing (2)
tests/bench/icons.bench.tstests/helpers.ts
… calls - Convert filesystem paths to file:// URLs via pathToFileURL for Windows compat - Add promise-based queue to serialize createInProcessRunner invocations, preventing re-entrancy issues with process-global patches Co-Authored-By: Claude <noreply@anthropic.com>
Remove the vitest bench + in-process runner setup that monkeypatched process globals (argv, exit, stdout, stderr) in favor of CodSpeed's official `codspeed exec` CLI which benchmarks the built CLI as a normal subprocess. - Delete createInProcessRunner and related types from tests/helpers.ts - Delete tests/bench/icons.bench.ts and vitest.bench.config.ts - Remove @codspeed/vitest-plugin dependency and test:bench script - Update CI workflow to use codspeed exec --mode walltime Co-Authored-By: Claude <noreply@anthropic.com>
The action already instruments commands via perf — running codspeed exec inside the action causes a sudo/perf conflict. Define benchmark targets in codspeed.yml and let the action (v4.9.0+) pick them up directly. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Summary
4b176df): Replace wildcardexport *re-exports in@kidd-cli/utils/fpwith explicit named exports, enabling bundler tree-shakingimport()the config loader (c12→yaml,jiti,confbox) in bothcli.tsandruntime.tsso the ~15K-line dependency chain is only parsed when actually running a command — not during--help/--versionrenderingThese two changes together eliminate the dominant startup cost identified in CPU profiling (42ms
compileSourceTextModulefor the 30K-line bundle).Files changed
packages/utils/src/fp/index.ts— explicit named re-exports instead ofexport *packages/core/src/cli.ts— dynamicimport('@kidd-cli/config/loader')insideresolveCommandsFromConfig()packages/core/src/runtime/runtime.ts— dynamicimport('@/lib/config/index.js')insideresolveConfig()Test plan
pnpm checkpasses (typecheck + lint + format)pnpm test --filter=@kidd-cli/core— all 856 tests pass--helpand--versionstartup time improvement with profiling