Replace Make + lerna with Bun scripts#309
Conversation
Delete Makefile, tools/build.mk, lerna.json, 28 per-package Makefiles, and dead electron/saucelabs workflows. Create tools/build.ts β shared esbuild build script that reads package.json for name/version/config. Each package gets a "build": "bun ../../tools/build.ts" script. Root package.json scripts replace all Make targets: bun run build β build all packages via bun --filter bun run test β vitest bun run lint β eslint bun run format β prettier bun run tsc β typecheck bun run clean β rm dist/ Eliminates make and lerna as dependencies. All commands are now `bun run <script>`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: π Files selected for processing (1)
β Files skipped from review due to trivial changes (1)
π WalkthroughWalkthroughReplaces Makefile/Lerna build orchestration with Bun workspaces and a shared Bun-based build script: removes many Makefiles and Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI / Developer
participant SetupBun as oven-sh/setup-bun
participant BunCLI as bun (cli)
participant Builder as tools/build.ts
participant Esbuild as bunx esbuild
participant FS as filesystem (dist/)
CI->>SetupBun: configure Bun using `.tool-versions`
CI->>BunCLI: run `bun run build` (per-package or CI step)
BunCLI->>Builder: execute `tools/build.ts`
Builder->>Esbuild: invoke esbuild builds (ESM / MJS / CJS [+ IIFE if browser])
Esbuild->>FS: write outputs (`dist/*.js`, metafiles)
Builder->>CI: exit status / logs
Estimated code review effortπ― 4 (Complex) | β±οΈ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
π₯ Pre-merge checks | β 2 | β 1β Failed checks (1 warning)
β Passed checks (2 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
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 |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d802855f12
βΉοΈ 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".
| @@ -1,8 +0,0 @@ | |||
|
|
|||
There was a problem hiding this comment.
Preserve docs prebuild target after removing package Makefiles
Removing this Makefile breaks the docs build path because tko.io/package.json still runs cd ../builds/knockout && make browser (and similarly for builds/reference) in prebuild; with the file deleted, that command now fails with No rule to make target 'browser'. This is user-facing in docs workflows and local docs development, and Bunβs script lifecycle runs pre<script> hooks before bun run build, so tko.io builds will fail until prebuild is migrated off make.
Useful? React with πΒ / π.
There was a problem hiding this comment.
Pull request overview
This PR migrates the monorepo build/test/lint tooling away from Make + Lerna and standardizes orchestration through Bun workspace scripts, with a shared tools/build.ts wrapper around esbuild.
Changes:
- Replace Make/Lerna orchestration with root
package.jsonscripts and per-workspacebuildscripts. - Introduce
tools/build.tsto centralize esbuild invocation for both package builds and browser bundle builds. - Update CI workflows and contributor docs/plans to reflect the new Bun-based commands; remove obsolete workflows/config.
Reviewed changes
Copilot reviewed 71 out of 72 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/template/Makefile | Removed obsolete Makefile template include. |
| tools/build.ts | New shared Bun-driven esbuild build script. |
| tools/build.mk | Deleted shared Make-based build configuration. |
| plans/replace-make-lerna.md | Added migration plan for replacing Make/Lerna with Bun scripts. |
| plans/modern-tooling.md | Updated phase tracking/status for modern-tooling initiative. |
| packages/utils/package.json | Added build script invoking tools/build.ts. |
| packages/utils/Makefile | Deleted per-package Makefile. |
| packages/utils.parser/package.json | Added build script invoking tools/build.ts. |
| packages/utils.parser/Makefile | Deleted per-package Makefile. |
| packages/utils.jsx/package.json | Added build script invoking tools/build.ts. |
| packages/utils.jsx/Makefile | Deleted per-package Makefile. |
| packages/utils.functionrewrite/package.json | Added build script invoking tools/build.ts. |
| packages/utils.functionrewrite/Makefile | Deleted per-package Makefile. |
| packages/utils.component/package.json | Added build script invoking tools/build.ts. |
| packages/utils.component/Makefile | Deleted per-package Makefile. |
| packages/provider/package.json | Added build script invoking tools/build.ts. |
| packages/provider/Makefile | Deleted per-package Makefile. |
| packages/provider.virtual/package.json | Added build script invoking tools/build.ts. |
| packages/provider.virtual/Makefile | Deleted per-package Makefile. |
| packages/provider.native/package.json | Added build script invoking tools/build.ts. |
| packages/provider.native/Makefile | Deleted per-package Makefile. |
| packages/provider.mustache/package.json | Added build script invoking tools/build.ts. |
| packages/provider.mustache/Makefile | Deleted per-package Makefile. |
| packages/provider.multi/package.json | Added build script invoking tools/build.ts. |
| packages/provider.multi/Makefile | Deleted per-package Makefile. |
| packages/provider.databind/package.json | Added build script invoking tools/build.ts. |
| packages/provider.databind/Makefile | Deleted per-package Makefile. |
| packages/provider.component/package.json | Added build script invoking tools/build.ts. |
| packages/provider.component/Makefile | Deleted per-package Makefile. |
| packages/provider.bindingstring/package.json | Added build script invoking tools/build.ts. |
| packages/provider.bindingstring/Makefile | Deleted per-package Makefile. |
| packages/provider.attr/package.json | Added build script invoking tools/build.ts. |
| packages/provider.attr/Makefile | Deleted per-package Makefile. |
| packages/observable/package.json | Added build script invoking tools/build.ts. |
| packages/observable/Makefile | Deleted per-package Makefile. |
| packages/lifecycle/package.json | Added build script invoking tools/build.ts. |
| packages/lifecycle/Makefile | Deleted per-package Makefile. |
| packages/filter.punches/package.json | Added build script invoking tools/build.ts. |
| packages/filter.punches/Makefile | Deleted per-package Makefile. |
| packages/computed/package.json | Added build script invoking tools/build.ts. |
| packages/computed/Makefile | Deleted per-package Makefile. |
| packages/builder/package.json | Added build script invoking tools/build.ts. |
| packages/builder/Makefile | Deleted per-package Makefile. |
| packages/binding.template/package.json | Added build script invoking tools/build.ts. |
| packages/binding.template/Makefile | Deleted per-package Makefile. |
| packages/binding.if/package.json | Added build script invoking tools/build.ts. |
| packages/binding.if/Makefile | Deleted per-package Makefile. |
| packages/binding.foreach/package.json | Added build script invoking tools/build.ts. |
| packages/binding.foreach/Makefile | Deleted per-package Makefile. |
| packages/binding.core/package.json | Added build script invoking tools/build.ts. |
| packages/binding.core/Makefile | Deleted per-package Makefile. |
| packages/binding.component/package.json | Added build script invoking tools/build.ts. |
| packages/binding.component/Makefile | Deleted per-package Makefile. |
| packages/bind/package.json | Added build script invoking tools/build.ts. |
| packages/bind/Makefile | Deleted per-package Makefile. |
| package.json | Added root Bun scripts; removed lerna dependency. |
| lerna.json | Deleted Lerna config. |
| bun.lock | Updated lockfile to reflect dependency removals/updates. |
| builds/reference/package.json | Added tko config + build script for browser-only build. |
| builds/reference/Makefile | Deleted build Makefile. |
| builds/knockout/package.json | Added tko config + build script for browser-only build (IIFE global ko). |
| builds/knockout/Makefile | Deleted build Makefile. |
| Makefile | Deleted root Make orchestrator. |
| AGENTS.md | Updated contributor commands to Bun equivalents (some Make/Lerna references remain). |
| .github/workflows/test-headless.yml | CI now runs bun run build instead of make. |
| .github/workflows/saucelabs.yaml | Deleted unused workflow. |
| .github/workflows/release.yml | Release workflow updated to use bun run build/test. |
| .github/workflows/publish-check.yml | Publish check workflow updated to use bun run build. |
| .github/workflows/main-build.yml | CI now runs bun run build instead of make. |
| .github/workflows/lint-and-typecheck.yml | CI now uses bun run format/lint/build instead of Make targets. |
| .github/workflows/electron.yaml | Deleted unused workflow. |
| .github/workflows/deploy-docs.yml | Docs workflow now builds monorepo via bun run build. |
| "exports": { | ||
| ".": { | ||
| "require": "./dist/index.cjs", | ||
| "import": "./dist/index.mjs" | ||
| } | ||
| }, | ||
| "tko": { | ||
| "iifeGlobalName": "ko", | ||
| "buildMode": "browser-only" | ||
| }, | ||
| "scripts": { | ||
| "build": "bun ../../tools/build.ts" | ||
| } |
There was a problem hiding this comment.
tko.buildMode is set to browser-only, but tools/build.ts does not emit dist/index.cjs or dist/index.mjs in that mode. This makes the current exports map (require/import -> ./dist/index.*) point at files that wonβt be produced by the build. Either generate the index artifacts even in browser-only mode, or update exports to reference the browser bundle outputs.
| }, | ||
| "bugs": { | ||
| "url": "https://github.com/knockout/tko/issues" | ||
| }, | ||
| "tko": { | ||
| "buildMode": "browser-only" | ||
| }, | ||
| "scripts": { | ||
| "build": "bun ../../tools/build.ts" | ||
| } |
There was a problem hiding this comment.
tko.buildMode is set to browser-only, but tools/build.ts does not emit dist/index.cjs/dist/index.mjs in that mode. The package exports currently reference ./dist/index.*, so consumers importing this package will fail unless those files are still produced. Consider either generating the index artifacts in browser-only mode or changing exports to reference the browser bundle outputs.
| - **Editor**: 2-space indentation for JS/TS, LF line endings | ||
| - See `.prettierrc` and `eslint.config.js` for full config | ||
|
|
||
| Run `make format-fix && make eslint-fix` before committing. |
There was a problem hiding this comment.
This instruction still tells contributors to run make format-fix && make eslint-fix, but make is removed in this PR and the equivalent commands are now bun run format:fix and bun run lint:fix. Update this line to prevent broken contributor guidance.
| Run `make format-fix && make eslint-fix` before committing. | |
| Run `bun run format:fix && bun run lint:fix` before committing. |
| ### `tools/build.ts` (~60 lines) | ||
|
|
||
| Shared build script that reads the local `package.json` to get name, version, | ||
| and optional TKO-specific config. Runs esbuild programmatically. | ||
|
|
There was a problem hiding this comment.
The plan says tools/build.ts runs esbuild βprogrammaticallyβ, but the implementation shells out to bunx esbuild. Please update the wording to match the actual approach (or adjust the implementation to use the JS API if thatβs the intent).
| ``` | ||
| packages/ # 26 modular @tko/* packages (all TypeScript) |
There was a problem hiding this comment.
The Project Structure section still references Lerna/lerna.json immediately above this code block, but Lerna is removed in this PR. Please update that description to reflect Bun workspaces and remove the stale lerna.json reference.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (2)
AGENTS.md (2)
13-19:β οΈ Potential issue | π‘ MinorRemove stale Lerna/
lerna.jsonwording in project structure intro.This now contradicts the migrated tooling and can misdirect contributors.
βοΈ Suggested doc fix
-Lerna monorepo with npm workspaces. Current version: see `lerna.json`. +Bun workspace monorepo. Current version: see root `package.json`.Based on learnings: Use Bun as the package manager and script runner instead of npm install.
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 13 - 19, Update the project intro in AGENTS.md to remove the stale "Lerna monorepo with npm workspaces" and any reference to `lerna.json`; replace it with the current tooling note that the repo uses Bun as the package manager and script runner (instead of npm), and clarify that packages/ contains the 26 TypeScript `@tko/`* packages, builds/ contains the distribution bundles, tools/ holds the shared build script, and tko.io/ is the docs site; ensure all mentions of "Lerna" and `lerna.json` are deleted or replaced and add a short line instructing contributors to use Bun for install/run tasks.
67-71:β οΈ Potential issue | π‘ MinorReplace leftover Make command in contributor instructions.
Line 70 still points to Make targets after moving to Bun scripts.
βοΈ Suggested doc fix
-Run `make format-fix && make eslint-fix` before committing. +Run `bun run format:fix && bun run lint:fix` before committing.Based on learnings: Use Bun as the package manager and script runner instead of npm install.
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 67 - 71, The contributor instructions still reference Make targets: replace the line containing "Run `make format-fix && make eslint-fix`" with the Bun-based script invocation (e.g., "Run `bun run format-fix && bun run eslint-fix`" or the project's exact Bun script names), updating the docs to state Bun is the package manager/script runner; change any phrasing that suggests make to instead reference Bun scripts and ensure the `.prettierrc`/`eslint.config.js` note remains.
π§Ή Nitpick comments (1)
tools/build.ts (1)
53-53: Metafile directory created outsidedist/.The
meta/directory is created in the package root, but this directory isn't listed in thefilesarray of package.json files (which only includedist/). This is fine if metafiles are only needed during development/CI analysis, but consider creatingdist/meta/instead if these files should be part of the published package or are expected to be gitignored alongside dist.β»οΈ Optional: Create metafiles inside dist/
- if (!existsSync('meta')) mkdirSync('meta', { recursive: true }) + if (!existsSync('dist/meta')) mkdirSync('dist/meta', { recursive: true })And update the
--metafilepaths on lines 59 and 62:- --metafile=meta/browser_min_meta.json + --metafile=dist/meta/browser_min_meta.jsonπ€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/build.ts` at line 53, Change the metafile directory from 'meta' to 'dist/meta' by replacing the literal 'meta' used in existsSync/mkdirSync with 'dist/meta' (so the directory is created under dist/), and update any build invocation flags that pass --metafile (the two occurrences currently supplying paths with 'meta') to point to the new dist/meta paths so generated metafiles are written into dist/meta instead of the repo root.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@builds/knockout/package.json`:
- Around line 61-64: The package.json sets the tko.buildMode to "browser-only"
which only emits dist/browser.js and dist/browser.min.js but the package exports
still reference "./dist/index.cjs" and "./dist/index.mjs"; update either the
"tko.buildMode" value to "default" or change the exports object to point to the
actual browser outputs (e.g., ./dist/browser.js and ./dist/browser.min.js or a
conditional export map) so exports match the artifacts produced by the tko build
step.
In `@tools/build.ts`:
- Around line 26-36: findSources() currently calls walk('src') and readdirSync
directly which throws ENOENT when src/ is missing; update findSources to guard
the initial walk call by checking existence (e.g., use existsSync or
fs.statSync) or wrap readdirSync in a try/catch inside walk to ignore ENOENT and
continue, so packages without a src/ directory return an empty sources array
instead of crashing; reference the findSources function and its inner walk
helper and the readdirSync call when making the change.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 13-19: Update the project intro in AGENTS.md to remove the stale
"Lerna monorepo with npm workspaces" and any reference to `lerna.json`; replace
it with the current tooling note that the repo uses Bun as the package manager
and script runner (instead of npm), and clarify that packages/ contains the 26
TypeScript `@tko/`* packages, builds/ contains the distribution bundles, tools/
holds the shared build script, and tko.io/ is the docs site; ensure all mentions
of "Lerna" and `lerna.json` are deleted or replaced and add a short line
instructing contributors to use Bun for install/run tasks.
- Around line 67-71: The contributor instructions still reference Make targets:
replace the line containing "Run `make format-fix && make eslint-fix`" with the
Bun-based script invocation (e.g., "Run `bun run format-fix && bun run
eslint-fix`" or the project's exact Bun script names), updating the docs to
state Bun is the package manager/script runner; change any phrasing that
suggests make to instead reference Bun scripts and ensure the
`.prettierrc`/`eslint.config.js` note remains.
---
Nitpick comments:
In `@tools/build.ts`:
- Line 53: Change the metafile directory from 'meta' to 'dist/meta' by replacing
the literal 'meta' used in existsSync/mkdirSync with 'dist/meta' (so the
directory is created under dist/), and update any build invocation flags that
pass --metafile (the two occurrences currently supplying paths with 'meta') to
point to the new dist/meta paths so generated metafiles are written into
dist/meta instead of the repo root.
πͺ 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 375c8aaf-04bd-4431-bacf-db36042d228e
β Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
π Files selected for processing (71)
.github/workflows/deploy-docs.yml.github/workflows/electron.yaml.github/workflows/lint-and-typecheck.yml.github/workflows/main-build.yml.github/workflows/publish-check.yml.github/workflows/release.yml.github/workflows/saucelabs.yaml.github/workflows/test-headless.ymlAGENTS.mdMakefilebuilds/knockout/Makefilebuilds/knockout/package.jsonbuilds/reference/Makefilebuilds/reference/package.jsonlerna.jsonpackage.jsonpackages/bind/Makefilepackages/bind/package.jsonpackages/binding.component/Makefilepackages/binding.component/package.jsonpackages/binding.core/Makefilepackages/binding.core/package.jsonpackages/binding.foreach/Makefilepackages/binding.foreach/package.jsonpackages/binding.if/Makefilepackages/binding.if/package.jsonpackages/binding.template/Makefilepackages/binding.template/package.jsonpackages/builder/Makefilepackages/builder/package.jsonpackages/computed/Makefilepackages/computed/package.jsonpackages/filter.punches/Makefilepackages/filter.punches/package.jsonpackages/lifecycle/Makefilepackages/lifecycle/package.jsonpackages/observable/Makefilepackages/observable/package.jsonpackages/provider.attr/Makefilepackages/provider.attr/package.jsonpackages/provider.bindingstring/Makefilepackages/provider.bindingstring/package.jsonpackages/provider.component/Makefilepackages/provider.component/package.jsonpackages/provider.databind/Makefilepackages/provider.databind/package.jsonpackages/provider.multi/Makefilepackages/provider.multi/package.jsonpackages/provider.mustache/Makefilepackages/provider.mustache/package.jsonpackages/provider.native/Makefilepackages/provider.native/package.jsonpackages/provider.virtual/Makefilepackages/provider.virtual/package.jsonpackages/provider/Makefilepackages/provider/package.jsonpackages/utils.component/Makefilepackages/utils.component/package.jsonpackages/utils.functionrewrite/Makefilepackages/utils.functionrewrite/package.jsonpackages/utils.jsx/Makefilepackages/utils.jsx/package.jsonpackages/utils.parser/Makefilepackages/utils.parser/package.jsonpackages/utils/Makefilepackages/utils/package.jsonplans/modern-tooling.mdplans/replace-make-lerna.mdtools/build.mktools/build.tstools/template/Makefile
π€ Files with no reviewable changes (33)
- packages/binding.core/Makefile
- packages/computed/Makefile
- packages/binding.component/Makefile
- packages/binding.if/Makefile
- packages/provider.bindingstring/Makefile
- packages/provider/Makefile
- .github/workflows/electron.yaml
- packages/binding.template/Makefile
- packages/observable/Makefile
- packages/filter.punches/Makefile
- packages/provider.multi/Makefile
- packages/provider.native/Makefile
- packages/bind/Makefile
- packages/provider.component/Makefile
- packages/provider.databind/Makefile
- packages/provider.virtual/Makefile
- packages/utils.component/Makefile
- lerna.json
- packages/utils.jsx/Makefile
- packages/provider.mustache/Makefile
- packages/utils.functionrewrite/Makefile
- .github/workflows/saucelabs.yaml
- builds/knockout/Makefile
- packages/provider.attr/Makefile
- packages/lifecycle/Makefile
- packages/utils/Makefile
- packages/binding.foreach/Makefile
- packages/builder/Makefile
- packages/utils.parser/Makefile
- Makefile
- tools/build.mk
- tools/template/Makefile
- builds/reference/Makefile
- Two-phase build: packages first, then builds (bun --filter doesn't toposort) - Change builds from browser-only to browser mode (ESM+CJS+MJS+IIFE) β builds/reference specs import from dist/index.mjs - Add globalThis to IIFE footer - Fix stale make reference in AGENTS.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Release workflow runs on bare ubuntu-latest (needs Node for npm trusted publishing), not the Playwright container. Instead of installing browsers (5+ min), just typecheck β full browser tests already passed in PR CI and main-build. Also remove stale make comments from codeql-analysis.yml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Restore apt-get install unzip in Playwright container workflows - Replace make browser with bun run build in tko.io prebuild script - Update AGENTS.md: LernaβBun workspaces Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use bun-version-file: .tool-versions in all setup-bun steps so CI uses the same Bun version as local development via mise/asdf. Prevents version drift between local and CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code already passed tsc in PR CI and main-build before tagging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| @@ -0,0 +1,70 @@ | |||
| /** | |||
- Use Bun.file, Bun.Glob, Bun.spawn instead of Node fs/child_process - Add shebang #!/usr/bin/env bun - Reduce llms.txt from 108 lines to 40 (concise like bun.sh/llms.txt) - Update plan to reflect Bun API usage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use Bun.spawn for esbuild invocation (Bun.$ raw doesn't work) - Parallelize ESM/MJS/CJS builds with Promise.all - Parallelize IIFE minified/unminified builds - Restore gotchas and esbuild-wasm snippet to llms.txt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All 5 builds (ESM, MJS, CJS + IIFE min/unmin) now run concurrently via a single Promise.all instead of two sequential groups. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build performanceClean build of all 27 packages now completes in 0.3 seconds (wall clock) - down from 10-15s: All 5 esbuild invocations per package (ESM, MJS, CJS + IIFE min/unmin for browser builds) run fully parallel via All 2679 tests pass, 42 skipped. |
There was a problem hiding this comment.
Actionable comments posted: 1
π§Ή Nitpick comments (1)
package.json (1)
19-19: Include browser build metadata in root clean target.Line 19 cleans
distandcoverage, buttools/build.tsalso emitsmeta/for browser builds. Consider cleaning that output too to avoid stale artifacts.Suggested diff
- "clean": "rm -rf packages/*/dist builds/*/dist coverage" + "clean": "rm -rf packages/*/dist builds/*/dist builds/*/meta coverage"π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 19, The root "clean" npm script currently removes packages/*/dist, builds/*/dist and coverage but omits browser build metadata emitted by tools/build.ts into meta/; update the "clean" script (the "clean" entry in package.json) to also remove the meta/ directories (e.g., add packages/*/meta and builds/*/meta or a top-level meta pattern) so browser build metadata is purged alongside dist and coverage to prevent stale artifacts.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 15-20: The fenced code block starting with ``` in AGENTS.md is
missing a language identifier (causing markdownlint MD040); update that opening
fence to include a neutral identifier such as "text" (e.g., change ``` to
```text) so the block is explicitly marked and the linter passes, keeping the
block contents unchanged.
---
Nitpick comments:
In `@package.json`:
- Line 19: The root "clean" npm script currently removes packages/*/dist,
builds/*/dist and coverage but omits browser build metadata emitted by
tools/build.ts into meta/; update the "clean" script (the "clean" entry in
package.json) to also remove the meta/ directories (e.g., add packages/*/meta
and builds/*/meta or a top-level meta pattern) so browser build metadata is
purged alongside dist and coverage to prevent stale artifacts.
πͺ 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: afe413fa-3d4a-489a-9aab-f5e94974dcbb
π Files selected for processing (15)
.github/workflows/codeql-analysis.yml.github/workflows/deploy-docs.yml.github/workflows/lint-and-typecheck.yml.github/workflows/main-build.yml.github/workflows/publish-check.yml.github/workflows/release.yml.github/workflows/test-headless.ymlAGENTS.mdbuilds/knockout/package.jsonbuilds/reference/package.jsonpackage.jsonplans/replace-make-lerna.mdtko.io/package.jsontko.io/public/llms.txttools/build.ts
π€ Files with no reviewable changes (1)
- .github/workflows/codeql-analysis.yml
β Files skipped from review due to trivial changes (3)
- tko.io/public/llms.txt
- builds/knockout/package.json
- plans/replace-make-lerna.md
π§ Files skipped from review as they are similar to previous changes (8)
- .github/workflows/main-build.yml
- .github/workflows/deploy-docs.yml
- .github/workflows/test-headless.yml
- .github/workflows/lint-and-typecheck.yml
- .github/workflows/release.yml
- builds/reference/package.json
- .github/workflows/publish-check.yml
- tools/build.ts
Build output comparison: branch vs mainMethodology
Results163 of 192 files are byte-for-byte identical (all ESM 29 files differ, all intentionally:
CJS size reductionThe old Make build bundled all IIFE buildsThe IIFE browser bundles remain fully self-contained (bundled). The only difference is ~49 bytes from adding |
CJS transitive dependency verificationMethodologyWith Tests
ResultsAll 25 individual packages load via The 2 builds packages ( |
Fixes markdownlint MD040 warning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Eliminate
makeandlernaas dependencies. All commands are nowbun run <script>.What changed
Deleted (72 files, -1501 lines):
Makefiletools/build.mk(shared build config)Makefilefileslerna.jsonlernafrom devDependencieselectron.yamlandsaucelabs.yamlworkflowstools/template/MakefileCreated:
tools/build.tsβ shared esbuild build script (~60 lines)plans/replace-make-lerna.mdβ migration planModified:
"build": "bun ../../tools/build.ts"builds/knockout/package.json:tko.iifeGlobalName: "ko",tko.buildMode: "browser-only"builds/reference/package.json:tko.buildMode: "browser-only"makeβbun run buildNew commands
Test plan
bun run buildβ 27 packages buildbun run testβ 2679 passed, 143 filesbun run tscβ 0 errorsπ€ Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation