chore(deps): update actions and dependencies, migrate to typescript 6#263
chore(deps): update actions and dependencies, migrate to typescript 6#263jbergstroem merged 6 commits intomainfrom
Conversation
Given the features I rely on, I've chosen to stick with Bun. This reverts commit e748d1f.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 37 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
WalkthroughThis pull request migrates the monorepo toolchain from pnpm to Bun as the primary package manager across all components. The changes include updating Renovate configuration to detect Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/elysia-sqlite/src/index-e2e.test.ts (1)
46-121:⚠️ Potential issue | 🟡 MinorAdd a per-record assertion to the contains operator test (line 101-110) to match the pattern of other filter tests.
The contains test only validates the count (394) and response metadata, but lacks a
.every()predicate check on the returned data. All other filter tests (status, AND operator, one-of, comparisons, range, nested query) verify that each record satisfies the filter condition. Add an assertion that every returned name contains ana(case-insensitive) to catch regressions if faker's seeded output changes again:expect(data.data.every((u: any) => u.name.toLowerCase().includes("a"))).toBe(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/elysia-sqlite/src/index-e2e.test.ts` around lines 46 - 121, In the "should filter with contains operator" test, add a per-record assertion after parsing `data` to mirror other tests: assert that every returned record in `data.data` satisfies the contains filter by checking `u.name.toLowerCase().includes("a")` (use the existing `response`/`data` variables and the `data.data.every((u: any) => ...)` pattern) so the test verifies each record as well as the count.
🧹 Nitpick comments (3)
bunfig.toml (1)
8-8: Nit: prefer array form forcoveragePathIgnorePatterns.Bun's bunfig accepts either a single glob or an array, but the array form is more idiomatic and avoids surprises if you later need to add patterns (e.g.
**/node_modules/*).-coveragePathIgnorePatterns = "**/dist/*" +coveragePathIgnorePatterns = ["**/dist/*"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bunfig.toml` at line 8, Change the bunfig setting coveragePathIgnorePatterns from a single-string value to an array so it's easier to extend later; locate the coveragePathIgnorePatterns entry in bunfig.toml and replace coveragePathIgnorePatterns = "**/dist/*" with an array form (e.g., coveragePathIgnorePatterns = ["**/dist/*"]) so additional patterns can be added without changing the type.tsconfig.base.json (1)
3-9: Explicitly settingmodule,moduleResolution,strict, andlibis optional for clarity.The base config relies on TS 6 implicit defaults, but TypeScript 6.0.3 is pinned via the workspace catalog, so the risk of accidental downgrade to
moduleResolution: "classic"is mitigated. However, explicit settings would make the intended configuration clearer and require no future changes if the catalog is updated.♻️ Optional explicit settings
{ "$schema": "https://json.schemastore.org/tsconfig", "compilerOptions": { "target": "ES2022", + "module": "ESNext", + "moduleResolution": "Bundler", + "strict": true, "skipLibCheck": true, "declaration": true, "emitDeclarationOnly": true, "isolatedModules": true }, "exclude": ["**/dist", "**/*.test.ts"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.base.json` around lines 3 - 9, The tsconfig.base.json compilerOptions omits explicit module, moduleResolution, strict, and lib settings which reduces clarity; update the "compilerOptions" block to explicitly set "module" (e.g., "ES2022" or "ESNext"), "moduleResolution" (e.g., "node"), "strict": true, and an appropriate "lib" array (e.g., ["es2022", "dom"] or similar) so the intended TS behavior is explicit and stable regardless of environment defaults..github/workflows/publish.yml (1)
67-72: Minor double-build when publishing@filtron/core.
bun run --filter=@filtron/core build(line 68) runs core's build, and then line 71bun run buildrebuilds it again whenmatrix.packageis core. Functionally fine, but you could gate the prebuild on dependents only:- if: matrix.package.name != 'core' run: bun run --filter=@filtron/core buildOtherwise everything else in this job — Bun pin via
bun-version-file: package.json,bun ci,bun pm packproducing the tgz consumed bynpm publish— looks correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish.yml around lines 67 - 72, The workflow currently always runs the prebuild step "bun run --filter=@filtron/core build" which causes a duplicate build when the current matrix package is core; change the job to conditionally run that prebuild only for dependents by adding an if: check (compare matrix.package.name to 'core') so that the step with "bun run --filter=@filtron/core build" only executes when matrix.package.name != 'core' and the later steps (cd "${{ matrix.package.dir }}"; bun run build; bun pm pack) remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/lint.yml:
- Around line 32-33: In the validate-renovate-config job add the missing with
block to the setup step that currently reads uses:
oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 so it uses the
repo-pinned Bun version; specifically, update the step for oven-sh/setup-bun to
include with: bun-version-file: package.json to match the lint and format jobs.
In `@bunfig.toml`:
- Line 4: Update all package.json manifests so their engines.bun requirement
matches the pinned packageManager Bun version; specifically, change engines.bun
from ">=1.1.0" (or any lower range) to ">=1.3.13" to ensure support for
install.linker = "isolated" (introduced in Bun v1.2.19). Locate the engines.bun
field in each package.json (root and workspace packages) and update its value to
">=1.3.13", and verify packageManager remains set to "bun@1.3.13" so CI and
local installs use the same Bun version.
In `@package.json`:
- Around line 49-53: Update the Bun engine floor from ">=1.1.0" to ">=1.2.0" in
the root package.json's "engines.bun" and in every workspace package manifest
that currently declares "bun": ">=1.1.0" (e.g., packages/core/package.json and
other packages), so the repo requires Bun 1.2+ to support bun.lock text
lockfile, workspaces.catalog, and catalog: protocol references; leave or keep
the existing "packageManager" value as-is unless you want to bump it separately.
In `@packages/benchmark/package.json`:
- Around line 40-44: Remove the redundant "bench" script or make it an explicit
alias to "bench:cpu" in package.json: the current "bench" and "bench:cpu" both
expand to "bun run bench:core && bun run bench:sql && bun run bench:js", so
either delete the "bench" entry or change its value to call "bench:cpu" (e.g.,
via "bun run bench:cpu") to avoid duplication while keeping "bench:cpu" as the
canonical CI-invoked script; update the entries referencing "bench" if any
elsewhere.
---
Outside diff comments:
In `@examples/elysia-sqlite/src/index-e2e.test.ts`:
- Around line 46-121: In the "should filter with contains operator" test, add a
per-record assertion after parsing `data` to mirror other tests: assert that
every returned record in `data.data` satisfies the contains filter by checking
`u.name.toLowerCase().includes("a")` (use the existing `response`/`data`
variables and the `data.data.every((u: any) => ...)` pattern) so the test
verifies each record as well as the count.
---
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 67-72: The workflow currently always runs the prebuild step "bun
run --filter=@filtron/core build" which causes a duplicate build when the
current matrix package is core; change the job to conditionally run that
prebuild only for dependents by adding an if: check (compare matrix.package.name
to 'core') so that the step with "bun run --filter=@filtron/core build" only
executes when matrix.package.name != 'core' and the later steps (cd "${{
matrix.package.dir }}"; bun run build; bun pm pack) remain unchanged.
In `@bunfig.toml`:
- Line 8: Change the bunfig setting coveragePathIgnorePatterns from a
single-string value to an array so it's easier to extend later; locate the
coveragePathIgnorePatterns entry in bunfig.toml and replace
coveragePathIgnorePatterns = "**/dist/*" with an array form (e.g.,
coveragePathIgnorePatterns = ["**/dist/*"]) so additional patterns can be added
without changing the type.
In `@tsconfig.base.json`:
- Around line 3-9: The tsconfig.base.json compilerOptions omits explicit module,
moduleResolution, strict, and lib settings which reduces clarity; update the
"compilerOptions" block to explicitly set "module" (e.g., "ES2022" or "ESNext"),
"moduleResolution" (e.g., "node"), "strict": true, and an appropriate "lib"
array (e.g., ["es2022", "dom"] or similar) so the intended TS behavior is
explicit and stable regardless of environment defaults.
🪄 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: CHILL
Plan: Pro
Run ID: 7452c64c-2ec7-4070-9d0a-ae6a4155bbfb
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.github/renovate.json.github/workflows/audit.yml.github/workflows/benchmarks.yml.github/workflows/lint.yml.github/workflows/publish.yml.github/workflows/test.yml.github/workflows/website.ymlbunfig.tomlexamples/elysia-sqlite/package.jsonexamples/elysia-sqlite/src/index-e2e.test.tsexamples/elysia-sqlite/tsconfig.jsonknip.jsonpackage.jsonpackages/benchmark/package.jsonpackages/benchmark/tsconfig.jsonpackages/core/package.jsonpackages/core/tsconfig.jsonpackages/js/package.jsonpackages/js/tsconfig.jsonpackages/sql/package.jsonpackages/sql/tsconfig.jsonpnpm-workspace.yamltsconfig.base.jsontsconfig.jsonwebsite/package.jsonwebsite/wrangler.json
💤 Files with no reviewable changes (2)
- examples/elysia-sqlite/tsconfig.json
- pnpm-workspace.yaml
1 participant
Revert the recent pnpm change and update all dependencies, including the major jump to Typescript 6.
Closes: #215