Upgrade dependencies #2224
Upgrade dependencies #2224
Conversation
WalkthroughAdds SyncChange and SyncChangeSet type definitions and a toSyncChangeSet converter; makes toDBOperationSet's txid optional and removes random txid generation; bumps multiple package/devDependency versions; changes TypeScript target to ES2020; converts many Rollup configs to ES modules and scoped plugins; adds a tool to fix duplicate .d.ts blocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
libs/dexie-cloud-common/src/SyncChangeSet.ts (1)
1-9: Consider more specific typing for upsert values.The
upsertarray uses genericobject[], which provides minimal type safety. Consider using generics or a more specific type if the structure of upserted objects is known.For example:
export type SyncChangeSet<T = object> = { [table: string]: { upsert?: T[]; update?: { [key: string]: { [keyPath: string]: any }; }; delete?: string[]; }; };This would allow callers to specify the expected object shape while maintaining backward compatibility with the default
objecttype.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
libs/dexie-cloud-common/package.json(2 hunks)libs/dexie-cloud-common/src/SyncChange.ts(1 hunks)libs/dexie-cloud-common/src/SyncChangeSet.ts(1 hunks)libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts(1 hunks)libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts(1 hunks)libs/dexie-cloud-common/src/index.ts(1 hunks)libs/dexie-cloud-common/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts (2)
libs/dexie-cloud-common/src/change-processing/DBKeyMutationSet.ts (1)
DBKeyMutationSet(3-5)libs/dexie-cloud-common/src/DBOperationsSet.ts (1)
DBOperationsSet(3-3)
libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts (2)
libs/dexie-cloud-common/src/change-processing/DBKeyMutationSet.ts (1)
DBKeyMutationSet(3-5)libs/dexie-cloud-common/src/SyncChangeSet.ts (1)
SyncChangeSet(1-9)
🔇 Additional comments (7)
libs/dexie-cloud-common/tsconfig.json (1)
6-6: LGTM!The TypeScript target update to es2020 aligns with the lib configuration and enables newer JavaScript features. This is a reasonable modernization.
libs/dexie-cloud-common/src/index.ts (1)
9-9: LGTM!The new exports for SyncChange and toSyncChangeSet properly extend the public API surface in a consistent manner.
Also applies to: 17-17
libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts (1)
12-60: LGTM!The conversion logic correctly transforms
DBKeyMutationSetintoSyncChangeSetby grouping mutations by table and operation type, then mapping them to the appropriate format (upsert values, update modification maps, and delete keys).libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts (2)
35-60: LGTM!The refactored function correctly handles optional
txidby passing it through (possibly undefined) to the operation objects. This simplifies the API by removing internal random txid generation.
11-11: This change is type-safe and does not require modifications.The
DBOperationCommoninterface (which all three operation types extend) already definestxidas an optional property:txid?: string | null. All three operation types (DBUpsertOperation,DBUpdateOperation,DBDeleteOperation) correctly inherit this optional txid property. The function properly assigns the optionaltxidparameter to each operation, and consumers oftoDBOperationSet(such asapplyOperation) do not depend on txid being present—they only process the operation type and keys/values/changeSpecs.libs/dexie-cloud-common/src/SyncChange.ts (1)
1-22: LGTM!The discriminated union pattern with the
typefield is well-structured and appropriate for representing different sync operations. The use ofobjectandanytypes provides flexibility for various data shapes, which is reasonable for a general-purpose sync library.libs/dexie-cloud-common/package.json (1)
25-25: lib0 update cannot be fully verified against public sources.The
lib0dependency was updated from 0.2.97 to 0.2.114. While the codebase uses stable core APIs (Encoder,Decoder, read/write functions for binary encoding/decoding), the lib0 package has no public changelog or release notes available. Without access to detailed changes across the 57 commits between these versions, breaking changes or API deprecations cannot be definitively confirmed or ruled out. Ensure this version was tested in your CI/build process before merging.
| "@types/jest": "^30.0.0", | ||
| "@types/node": "^24.10.1", | ||
| "jest": "^30.2.0", | ||
| "ts-jest": "^29.4.6", | ||
| "typescript": "^5.9.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jest 30 breaking changes migration from Jest 29
💡 Result:
Below are the main breaking changes when upgrading from Jest 29 → Jest 30 and practical migration steps.
Key breaking changes (summary)
- Node support: Node 14, 16, 19, 21 dropped — minimum supported Node is now 18.x. (jestjs.io)
- TypeScript: minimum TypeScript required is 5.4. (jestjs.io)
- jsdom: jest-environment-jsdom upgraded (jsdom v21 → v26) — DOM behavior/warnings may change. (jestjs.io)
- Expect API changes: various expect aliases removed (use eslint-plugin-jest autofixer to migrate). (jestjs.io)
- Object matchers: non-enumerable properties are now excluded from object matchers (e.g., toEqual) by default. (jestjs.io)
- ESM / file types: .mts and .cts are supported by default; better native-ESM handling (import.meta, file://). (jestjs.io)
- CLI/config rename: --testPathPattern → --testPathPatterns; globalsCleanupMode renamed to globalsCleanup; other flag renames. (github.com)
- Snapshots: snapshot printing changed — many snapshots may need updating (messages about deprecated goo.gl links were handled). (jestjs.io)
- Bundling / exports: Jest packages are bundled and publish proper "exports"; deep/unsupported internal imports will break — rely only on public APIs. (fossies.org)
- Promise handling: improved handling of promises that are rejected then later caught (avoids false positives). (jestjs.io)
Practical migration steps
- Upgrade runtime and toolchain
- Move CI/dev machines to Node 18+ and update TypeScript to ≥5.4 if you use TS. (jestjs.io)
- Update Jest package
- npm install jest@^30 and update related @jest/* packages and @types/jest. (jestjs.io)
- Fix test & code API changes
- Run eslint-plugin-jest autofix to replace removed expect aliases; manually review remaining expect usages. (jestjs.io)
- Update snapshots
- Run tests with --updateSnapshot and review snapshot diffs (printing changes may require review). (jestjs.io)
- Search-and-replace deprecated flags / config keys
- Replace renamed CLI flags and config fields (e.g., testPathPattern → testPathPatterns, globalsCleanupMode → globalsCleanup). (github.com)
- Remove deep/internal imports
- Replace any code that imports Jest internals (deep paths) with public APIs; open issues/PRs if you need an API exposed. (fossies.org)
- Test DOM differences
- If you rely on jsdom behavior, run DOM tests and consult jsdom v26 notes for any required adjustments. (jestjs.io)
- Run full test suite and CI
- Fix failures (expect matcher changes, promise timing, snapshot diffs), then pin the new versions in lockfiles. (jestjs.io)
References / official docs
- Jest 30 announcement & blog post. (jestjs.io)
- Upgrading from Jest 29 → 30 (upgrade guide). (jestjs.io)
- Jest GitHub releases / changelog. (github.com)
If you want, I can: (a) produce an automated checklist or codemods (eslint/ripgrep commands) to update flags and expect aliases in your repo, or (b) scan a sample of your codebase for likely breakages — tell me which you prefer.
Citations:
- 1: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 2: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 3: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 4: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 5: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 6: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 7: https://github.com/jestjs/jest/releases?utm_source=openai
- 8: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 9: https://fossies.org/linux/jest/website/versioned_docs/version-30.0/UpgradingToJest30.md?utm_source=openai
- 10: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 11: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 12: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 13: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 14: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 15: https://github.com/jestjs/jest/releases?utm_source=openai
- 16: https://fossies.org/linux/jest/website/versioned_docs/version-30.0/UpgradingToJest30.md?utm_source=openai
- 17: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 18: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 19: https://jestjs.io/blog/2025/06/04/jest-30?utm_source=openai
- 20: https://jestjs.io/docs/upgrading-to-jest30?utm_source=openai
- 21: https://github.com/jestjs/jest/releases?utm_source=openai
🌐 Web query:
@types/node version 24 breaking changes from version 18
💡 Result:
Short answer: @types/node doesn't publish a separate "breaking‑changes" changelog — its majors track Node.js majors, so major bumps from 18→24 reflect the API/behavior changes in Node 24 and some stricter or reorganized type surface in DefinitelyTyped. See below for the important effects and where to look.
What changed / what breaks when moving @types/node 18 → 24
- No single curated list in @types; changes follow Node.js 24 API updates (new globals, removed/deprecated APIs, runtime behavior changes). Test and fix code that relied on removed or deprecated Node APIs. [1][2]
- New globals and platform features in Node 24 (e.g., URLPattern global, Float16Array, AsyncContext/AsyncLocalStorage behavior changes) are reflected in the types — you may get new types or different overloads. [1]
- Some older runtime APIs deprecated/removed in Node 24 (examples: legacy fs/tls/url APIs, internal bindings) will no longer appear (or may be narrowed) in the types — code using them may need edits. Test runtime‑coupled code and native addons (V8 bump requires rebuilds). [1][3]
- TypeScript/tooling: newer @types/node 24 may require newer TS versions or change types that were previously permissive (narrower signatures, stricter nullability), causing compile errors you must fix. DefinitelyTyped discussants recommend pinning the @types major to your Node major. [4][2]
Practical migration steps
- Pin @types/node to the Node major you run in CI (e.g. use @types/node@18 for Node 18). DefinitelyTyped recommends matching the targeted Node major. [4]
- Run full type check and tests after bump; rebuild native modules if upgrading Node runtime (V8 ABI change). [1][3]
- If you need specific removals/additions, inspect the Node 24 release notes and the @types/node package diff/commits in DefinitelyTyped for exact typing changes. [1][4]
References
- Node.js 24 release notes / breaking changes. [1]
- @types/node release history (npm / release lists) and recommendation to align types with Node major. [2][4]
- DefinitelyTyped repo / discussion about @types/node versioning and no dedicated changelog. [4]
If you want, I can: (a) list concrete removed/changed Node APIs between Node 18 and Node 24 that affect typings, or (b) fetch the specific @types/node commit/PR diffs between the 18.x and 24.x tags so you can inspect exact typing changes. Which do you prefer?
Verify major version updates for testing dependencies introduce no test failures.
These updates include significant breaking changes that require test suite validation:
- Jest 29 → 30: Expect API aliases removed, object matcher behavior changed (non-enumerable properties now excluded by default), snapshot printing changed, jsdom upgraded (v21 → v26), CLI flag renames required, and bundled exports break deep internal imports. TypeScript ≥5.4 is now required (your package.json specifies ^5.9.3, which is compatible).
- @types/node 18 → 24: Type surface reflects Node 24 API changes including new globals (URLPattern, Float16Array), deprecated/removed APIs, and AsyncContext behavior changes. Ensure tests pass after upgrade and verify no code relies on removed Node APIs.
Run the full test suite, update snapshots as needed, and validate expect() calls and configuration flags are compatible with Jest 30 before merging.
🤖 Prompt for AI Agents
In libs/dexie-cloud-common/package.json around lines 18 to 22, the test-related
dependencies were bumped (jest to ^30.x, @types/node to ^24.x, ts-jest and
TypeScript versions updated) but the test suite may break due to Jest 30 and
Node typings changes; run the full test suite locally/CI, update failing
snapshots, fix any expect() calls or matcher usage that rely on old Jest
behavior (handle non-enumerable properties, updated snapshot printing,
CLI/config flags), and address TypeScript/@types/node-induced type errors or
code that relies on removed Node APIs; after fixes, commit updated snapshots and
any code/test adjustments and re-run tests until all pass.
| import { DBDeleteOperation, DBOperation, DBUpdateOperation, DBUpsertOperation } from "../DBOperation.js"; | ||
| import { DBOperationsSet } from "../DBOperationsSet.js"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused imports.
The imports DBDeleteOperation, DBOperation, DBUpdateOperation, DBUpsertOperation, and DBOperationsSet are not used in this file.
Apply this diff to remove the unused imports:
-import { DBDeleteOperation, DBOperation, DBUpdateOperation, DBUpsertOperation } from "../DBOperation.js";
-import { DBOperationsSet } from "../DBOperationsSet.js";
import { SyncChangeSet } from "../SyncChangeSet.js";
import { DBKeyMutationSet } from "./DBKeyMutationSet.js";🤖 Prompt for AI Agents
In libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts around lines
1 to 2, the imported symbols DBDeleteOperation, DBOperation, DBUpdateOperation,
DBUpsertOperation, and DBOperationsSet are unused; remove those unused imports
from the top of the file (either delete the entire import lines or reduce the
import lists to only the actually used symbols) so the file no longer imports
unused identifiers.
| const map: { | ||
| [table: string]: { [opType: string]: { key: any, val?: any, mod?: any }[] }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find DBKeyMutation and related type definitions
echo "=== Searching for DBKeyMutation type definition ==="
rg -n "type DBKeyMutation" libs/dexie-cloud-common/src/ -A 5
echo -e "\n=== Searching for DBKeyMutationSet type definition ==="
rg -n "type DBKeyMutationSet\|interface DBKeyMutationSet" libs/dexie-cloud-common/src/ -A 5
echo -e "\n=== Searching for SyncChangeSet type definition ==="
rg -n "type SyncChangeSet\|interface SyncChangeSet" libs/dexie-cloud-common/src/ -A 10
echo -e "\n=== Reading the toSyncChangeSet.ts file ==="
wc -l libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.tsRepository: dexie/Dexie.js
Length of output: 1130
🏁 Script executed:
#!/bin/bash
# Read the relevant sections of toSyncChangeSet.ts
head -50 libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.tsRepository: dexie/Dexie.js
Length of output: 2023
🏁 Script executed:
#!/bin/bash
# Search for how keys are populated in the map
rg -n "map\[" libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts -B 2 -A 2Repository: dexie/Dexie.js
Length of output: 395
🏁 Script executed:
#!/bin/bash
# Check SyncChangeSet type definition
rg -n "type SyncChangeSet\|interface SyncChangeSet" libs/dexie-cloud-common/src/ -A 15Repository: dexie/Dexie.js
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Also check where toSyncChangeSet is used to understand if there's validation
rg -n "toSyncChangeSet" libs/dexie-cloud-common/src/ -B 2 -A 2Repository: dexie/Dexie.js
Length of output: 1064
🏁 Script executed:
#!/bin/bash
# Find the SyncChangeSet file
find libs/dexie-cloud-common/src -name "*SyncChangeSet*" -type fRepository: dexie/Dexie.js
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Search for SyncChangeSet definition without type/interface keywords in case it's a different format
rg -n "SyncChangeSet" libs/dexie-cloud-common/src/ --type=ts | head -20Repository: dexie/Dexie.js
Length of output: 890
🏁 Script executed:
#!/bin/bash
# Read the SyncChangeSet.ts file
cat libs/dexie-cloud-common/src/SyncChangeSet.tsRepository: dexie/Dexie.js
Length of output: 233
Use string instead of any for the key type in the temporary map.
The input inSet is of type DBKeyMutationSet, where keys are explicitly typed as string. When the keys are later used in SyncChangeSet, they must be strings (for the delete array and update map keys). The temporary map on line 14-15 should reflect this:
const map: {
[table: string]: { [opType: string]: { key: string, val?: any, mod?: any }[] };
} = {};This improves type safety and clarity without changing behavior.
🤖 Prompt for AI Agents
In libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts around lines
14-15, the temporary map's key property is typed as any but should be string to
match DBKeyMutationSet and downstream SyncChangeSet usage; change the map's type
so each entry's key is string (e.g., { [table: string]: { [opType: string]: {
key: string, val?: any, mod?: any }[] } }) and keep the rest of the logic
unchanged.
dfahlander
force-pushed
the
common-types-for-webhooks
branch
from
b275792 to
c014e20
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts (1)
36-53: Consider adding a default case to the switch statement.The switch currently handles "ups", "upd", and "del" operation types. Consider adding a default case to detect and handle unexpected operation types, which would make the function more robust if new operation types are added in the future.
Apply this diff to add a default case:
case "del": { resultEntry.delete = muts.map(mut => mut.key); break; } + default: { + console.warn(`Unknown operation type: ${optype}`); + break; + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
libs/dexie-cloud-common/package.json(2 hunks)libs/dexie-cloud-common/src/SyncChange.ts(1 hunks)libs/dexie-cloud-common/src/SyncChangeSet.ts(1 hunks)libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts(1 hunks)libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts(1 hunks)libs/dexie-cloud-common/src/index.ts(1 hunks)libs/dexie-cloud-common/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/dexie-cloud-common/src/SyncChangeSet.ts
- libs/dexie-cloud-common/tsconfig.json
- libs/dexie-cloud-common/src/SyncChange.ts
🧰 Additional context used
🧬 Code graph analysis (2)
libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts (2)
libs/dexie-cloud-common/src/change-processing/DBKeyMutationSet.ts (1)
DBKeyMutationSet(3-5)libs/dexie-cloud-common/src/SyncChangeSet.ts (1)
SyncChangeSet(1-9)
libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts (2)
libs/dexie-cloud-common/src/change-processing/DBKeyMutationSet.ts (1)
DBKeyMutationSet(3-5)libs/dexie-cloud-common/src/DBOperationsSet.ts (1)
DBOperationsSet(3-3)
🔇 Additional comments (4)
libs/dexie-cloud-common/src/change-processing/toDBOperationSet.ts (1)
11-59: Optionaltxidsemantics – verify downstream type & behavior expectationsMaking
txidoptional and passing it through directly (txid?: string→{ ..., txid }) looks consistent, but it does change guarantees: operations can now carrytxid: undefinedinstead of an always‑present string.Please double‑check that:
DBUpsertOperation,DBUpdateOperation, andDBDeleteOperationdefinetxidas optional (or acceptstring | undefined) so the object literals type‑check cleanly.- All consumers of
DBOperationsSettolerate a missing/undefinedtxidand don’t rely on the previous “always non‑empty string” behavior for idempotency, grouping, or logging.If any caller still assumes a guaranteed transaction id, you may need to either keep a defaulting behavior there or adjust those callers.
libs/dexie-cloud-common/src/index.ts (1)
9-9: LGTM! New public exports added correctly.The new exports for
SyncChange.jsandtoSyncChangeSet.jscleanly extend the public API with the webhook-related types and conversion utilities.Also applies to: 17-17
libs/dexie-cloud-common/src/change-processing/toSyncChangeSet.ts (1)
12-59: Transformation logic is correct and well-structured.The function correctly transforms
DBKeyMutationSettoSyncChangeSetby:
- Building an intermediate map grouped by table and operation type (lines 17-23)
- Converting each operation type to the appropriate output format (lines 36-53):
- "ups" →
upsertarray of values- "upd" →
updatemap of key to field modifications- "del" →
deletearray of keysThe logic aligns with the type definitions and handles the transformation accurately.
libs/dexie-cloud-common/package.json (1)
25-25: The lib0 upgrade from ^0.2.97 to ^0.2.114 is safe. While this version includes a breaking change to the schema API (s.null / s.void renamed), dexie-cloud-common only uses lib0's encoding/decoding utilities, which are unaffected.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
addons/dexie-cloud/tools/build-configs/rollup.config.mjs (1)
9-23: Align with the codebase pattern: replace Import Attributes withreadFileSync+JSON.parseThe
import ... with { type: 'json' }syntax requires Node.js v18.20.0, v20.10.0, or v21.0.0+. No version pinning is defined in this repository, and all other build configs usereadFileSync+JSON.parsefor package.json imports. For consistency and broader compatibility, switch to the established pattern.Also remove the redundant
import * as fson line 11 sincereadFileSyncandwriteFileSyncare already imported on line 5.- // @ts-ignore: requires tsconfig settings that we don't need for the web build but is ok here in the build config. - import pkg from '../../package.json' with { type: 'json' }; - import * as fs from 'fs'; + // @ts-ignore: requires tsconfig settings that we don't need for the web build but is ok here in the build config. + const pkg = JSON.parse(readFileSync(new URL('../../package.json', import.meta.url), 'utf-8'));addons/dexie-cloud/package.json (1)
48-50: Updateengines.nodeto>=18to match Rollup 4.x requirement.Rollup 4.x (including v4.53.3) requires Node.js >= 18.0.0. The current setting
"node": ">=14"is incompatible and already contradicts the dev dependency@types/node@^18.11.18."engines": { - "node": ">=14" + "node": ">=18" },package.json (1)
121-145: Update CI configuration to Node.js 20+ to support cross-env 10.1.0, or downgrade cross-env to a v9.x version compatible with Node 18.The CI workflow (
.github/workflows/main.yml) is configured for Node 18, but cross-env 10.1.0 requires Node >= 20 (a breaking change introduced in v10.0.0). Either upgrade the CI Node version or pin cross-env to a v9.x release that supports Node 18.The Rollup 4.53.3 and plugin versions are compatible, and dts-bundle-generator 9.5.1 supports TypeScript >= 5.0.2, so those combinations are safe.
Separately, qunit 2.10.0 and qunitjs 1.23.1 are pinned without semver ranges—clarify if this is intentional or should use caret ranges for consistency with other dependencies.
♻️ Duplicate comments (1)
libs/dexie-cloud-common/package.json (1)
18-22: Checkts-jestcompatibility with Jest 30 (majors look mismatched)
Withjest@^30.2.0,ts-jest@^29.4.6is a likely incompatibility—please verify the supported Jest range for that ts-jest version and bump accordingly if needed.ts-jest compatibility matrix: which ts-jest major version supports Jest 30.x?
🧹 Nitpick comments (10)
addons/dexie-cloud/src/sync/performGuardedJob.ts (1)
8-14: Avoid@ts-expect-errorhere if a small typed wrapper will do (and guard against “unused expect-error” failures).
Since TS/DOM typings can change, prefer making the callback type explicit instead of suppressing inference errors, e.g. wrapjobin a callback typed asLockGrantedCallback<Promise<T>>(or the equivalent in your TS lib.dom). If you keep@ts-expect-error, consider adding a short note on which TS version / lib.dom signature triggers it, because future upgrades may make it unused and break CI.- // @ts-expect-error - LockManager callback type inference issue with generics - return navigator.locks.request(db.name + '|' + jobName, job); + // Make callback type explicit to avoid TS inference issues across TS/lib.dom versions. + const cb: LockGrantedCallback<Promise<T>> = () => job(); + return navigator.locks.request(db.name + '|' + jobName, cb);addons/Dexie.Syncable/tools/build-configs/rollup.config.mjs (1)
4-9: Nice ESM-friendly__dirnamesetup; consider using/removing the unusedversionbinding.If
versionisn’t used by this config (now or soon), dropping it avoids future “why is this here?” drift; otherwise wire it into the banner/replace step here for locality.addons/Dexie.Observable/tools/build-configs/rollup.config.mjs (1)
4-9:versionis computed but unused—either remove or use it intentionally
Not a blocker, but dead variables in build configs tend to confuse future maintenance.tools/build-configs/rollup.modern.config.mjs (1)
9-13:versionis unused in this config
Optional: drop it to reduce noise, unless you intend to wire it into the banner.addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.mjs (1)
9-23: Preferpath.resolve(__dirname, ...)for input/output paths to avoid cwd sensitivity
Matches the pattern used by other.mjsrollup configs in this PR and makes the build more robust.tools/build-configs/rollup.modern.config.js (1)
8-9: Consider removing or usingversionto avoid dead config state.
versionis read but not used anywhere in this config.If it’s only there for future use, I’d drop it; otherwise embed it into the banner (or similar) intentionally.
Also applies to: 14-35
addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs (1)
5-32: Optional: consider deduplicatingonwarn+ERRORS_TO_IGNOREacross configs.There’s a lot of repeated warning formatting across multiple Rollup configs in this PR; a tiny shared helper could reduce maintenance.
tools/build-configs/rollup.umd.config.mjs (1)
11-13:versionis unused—either wire it in or remove it.tools/build-configs/rollup.config.mjs (1)
11-13: Optional: drop or useversionto avoid dead code.package.json (1)
87-88: Clarify the purpose of --no-check flag and fix-dts-duplicates.js tool.Adding
--no-checkto dts-bundle-generator disables type checking during bundling, which may improve build speed but could hide type errors. The new post-processing scriptfix-dts-duplicates.js(line 88) suggests duplicate type declarations are being generated.Please clarify:
- Why --no-check was introduced (performance optimization or necessity?)
- What specific duplicate declarations the fix-dts-duplicates.js tool removes
- Whether this approach is safe or if type checking should remain enabled
Provide the fix-dts-duplicates.js file for review to verify it doesn't inadvertently remove valid type declarations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
addons/Dexie.Observable/package.json(3 hunks)addons/Dexie.Observable/tools/build-configs/rollup.config.mjs(1 hunks)addons/Dexie.Observable/tools/build-configs/rollup.tests.config.js(1 hunks)addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs(1 hunks)addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.js(1 hunks)addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.mjs(1 hunks)addons/Dexie.Syncable/package.json(2 hunks)addons/Dexie.Syncable/tools/build-configs/rollup.config.mjs(1 hunks)addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.js(1 hunks)addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.mjs(1 hunks)addons/dexie-cloud/package.json(1 hunks)addons/dexie-cloud/src/sync/performGuardedJob.ts(1 hunks)addons/dexie-cloud/tools/build-configs/rollup.config.mjs(4 hunks)addons/dexie-cloud/tools/build-configs/rollup.test.unit.config.js(1 hunks)addons/dexie-export-import/package.json(1 hunks)addons/dexie-export-import/tools/build-configs/rollup.config.mjs(1 hunks)addons/dexie-export-import/tools/build-configs/rollup.tests.config.mjs(1 hunks)addons/y-dexie/package.json(2 hunks)addons/y-dexie/tools/build-configs/rollup.config.mjs(1 hunks)addons/y-dexie/tools/build-configs/rollup.test.unit.config.js(1 hunks)libs/dexie-cloud-common/package.json(2 hunks)libs/dexie-react-hooks/package.json(2 hunks)package.json(2 hunks)test/integrations/test-dexie-relationships/package.json(1 hunks)test/karma-env.js(1 hunks)test/karma.common.js(2 hunks)tools/build-configs/rollup.config.js(1 hunks)tools/build-configs/rollup.config.mjs(1 hunks)tools/build-configs/rollup.modern.config.js(1 hunks)tools/build-configs/rollup.modern.config.mjs(1 hunks)tools/build-configs/rollup.tests.config.js(1 hunks)tools/build-configs/rollup.tests.config.mjs(1 hunks)tools/build-configs/rollup.umd.config.js(1 hunks)tools/build-configs/rollup.umd.config.mjs(1 hunks)tools/fix-dts-duplicates.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- addons/y-dexie/package.json
🧰 Additional context used
🧬 Code graph analysis (8)
addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs (2)
addons/dexie-export-import/tools/build-configs/rollup.config.mjs (1)
ERRORS_TO_IGNORE(17-19)libs/dexie-react-hooks/rollup.config.mjs (1)
ERRORS_TO_IGNORE(9-11)
addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.mjs (8)
addons/Dexie.Observable/tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.mjs (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/dexie-export-import/tools/build-configs/rollup.config.mjs (1)
ERRORS_TO_IGNORE(17-19)addons/dexie-export-import/tools/build-configs/rollup.tests.config.mjs (1)
ERRORS_TO_IGNORE(12-14)libs/dexie-react-hooks/rollup.config.mjs (1)
ERRORS_TO_IGNORE(9-11)
addons/Dexie.Syncable/tools/build-configs/rollup.config.mjs (2)
addons/Dexie.Observable/tools/build-configs/rollup.config.mjs (4)
__filename(6-6)__dirname(7-7)packageJson(8-8)version(9-9)addons/dexie-export-import/tools/build-configs/rollup.config.mjs (3)
__filename(10-10)__dirname(11-11)JSON(15-15)
addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.mjs (8)
addons/Dexie.Observable/tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(5-7)addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.mjs (1)
ERRORS_TO_IGNORE(5-7)addons/dexie-export-import/tools/build-configs/rollup.config.mjs (1)
ERRORS_TO_IGNORE(17-19)addons/dexie-export-import/tools/build-configs/rollup.tests.config.mjs (1)
ERRORS_TO_IGNORE(12-14)libs/dexie-react-hooks/rollup.config.mjs (1)
ERRORS_TO_IGNORE(9-11)
tools/build-configs/rollup.umd.config.mjs (3)
tools/build-configs/rollup.config.mjs (5)
__filename(9-9)__dirname(10-10)packageJson(11-11)version(12-12)ERRORS_TO_IGNORE(14-16)tools/build-configs/rollup.tests.config.mjs (3)
__filename(7-7)__dirname(8-8)ERRORS_TO_IGNORE(10-13)tools/build-configs/rollup.umd.config.js (2)
version(8-8)ERRORS_TO_IGNORE(10-12)
addons/Dexie.Observable/tools/build-configs/rollup.config.mjs (3)
addons/Dexie.Syncable/tools/build-configs/rollup.config.mjs (4)
__filename(6-6)__dirname(7-7)packageJson(8-8)version(9-9)addons/dexie-export-import/tools/build-configs/rollup.config.mjs (3)
__filename(10-10)__dirname(11-11)JSON(15-15)addons/dexie-export-import/tools/build-configs/rollup.tests.config.mjs (2)
__filename(9-9)__dirname(10-10)
tools/build-configs/rollup.config.mjs (6)
tools/build-configs/rollup.tests.config.mjs (3)
__filename(7-7)__dirname(8-8)ERRORS_TO_IGNORE(10-13)addons/dexie-export-import/tools/build-configs/rollup.config.mjs (1)
JSON(15-15)tools/build-configs/rollup.config.js (2)
version(8-8)ERRORS_TO_IGNORE(10-12)tools/build-configs/rollup.modern.config.js (2)
version(8-8)ERRORS_TO_IGNORE(10-12)tools/build-configs/rollup.umd.config.js (2)
version(8-8)ERRORS_TO_IGNORE(10-12)tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(6-9)
tools/build-configs/rollup.tests.config.mjs (4)
tools/build-configs/rollup.config.mjs (3)
__filename(9-9)__dirname(10-10)ERRORS_TO_IGNORE(14-16)tools/build-configs/rollup.modern.config.mjs (3)
__filename(9-9)__dirname(10-10)ERRORS_TO_IGNORE(14-16)tools/build-configs/rollup.umd.config.mjs (3)
__filename(9-9)__dirname(10-10)ERRORS_TO_IGNORE(14-16)tools/build-configs/rollup.tests.config.js (1)
ERRORS_TO_IGNORE(6-9)
⏰ 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). (4)
- GitHub Check: test (libs/dexie-react-hooks/test)
- GitHub Check: test (test)
- GitHub Check: test (addons/Dexie.Syncable/test)
- GitHub Check: test (addons/Dexie.Observable/test)
🔇 Additional comments (26)
addons/dexie-cloud/src/sync/performGuardedJob.ts (1)
8-11: Guard clause looks correct and keeps behavior consistent in non-browser/unsupported environments.tools/build-configs/rollup.umd.config.js (1)
2-2: LGTM! Plugin migration to scoped package.The migration from
rollup-plugin-node-resolveto@rollup/plugin-node-resolveis correct and aligns with the modern, maintained Rollup plugin ecosystem. The plugin usage and configuration remain functionally equivalent.tools/build-configs/rollup.config.js (1)
2-2: LGTM! Consistent plugin migration.The migration to
@rollup/plugin-node-resolveis correct and consistent with the repository-wide update to scoped Rollup plugins.addons/Dexie.Observable/tools/build-configs/rollup.tests.config.js (1)
2-3: LGTM! Test configuration plugin migration.Both plugin migrations are correct:
rollup-plugin-commonjs→@rollup/plugin-commonjsrollup-plugin-node-resolve→@rollup/plugin-node-resolveThe plugin usage remains unchanged and functionally equivalent.
tools/build-configs/rollup.tests.config.js (1)
2-3: LGTM! Consistent test configuration migration.The plugin migrations align with the repository-wide update to scoped Rollup plugins. Usage remains functionally equivalent.
addons/dexie-export-import/tools/build-configs/rollup.config.mjs (1)
4-5: LGTM! ES module configuration updated.The plugin migrations to
@rollup/plugin-commonjsand@rollup/plugin-node-resolveare correct and consistent with the scoped package migration across the repository.addons/y-dexie/tools/build-configs/rollup.config.mjs (1)
10-10: LGTM! Updated to standard import attributes syntax.The change from
assert { type: 'json' }towith { type: 'json' }adopts the newer ECMAScript import attributes standard that superseded the import assertions proposal.Note: The
withsyntax requires Node.js 20.10+ or 21.0+ (with--experimental-import-attributesin earlier versions). Please verify the minimum Node.js version requirements are documented if not already.test/integrations/test-dexie-relationships/package.json (1)
18-21: LGTM! Test dependencies added.The QUnit test dependencies have been added consistently with other test packages in the repository.
Note: Same version consideration as in addons/dexie-export-import/package.json applies here (qunit 2.10.0 and qunitjs 1.23.1 are older versions).
test/karma.common.js (1)
7-9: Manual QUnit wiring is reasonable, but verify karma-qunit works withoutframeworks: ['qunit']across all suites.In particular, ensure the adapter doesn’t assume Karma’s
frameworkshook ran (or auto-load a different QUnit), and thatnode_modules/qunitjs/qunit/qunit.jsexists in all package install contexts (pnpm workspace hoisting can differ).Also applies to: 34-37
addons/dexie-cloud/tools/build-configs/rollup.test.unit.config.js (1)
7-7: This syntax is appropriate for the project's Node 18 CI requirement. Theimport ... with { type: 'json' }syntax requires Node v18.20.0+, which your CI already uses. No compatibility concern exists here unless the project explicitly intends to support older Node versions—which is not indicated in the codebase.Likely an incorrect or invalid review comment.
addons/dexie-export-import/tools/build-configs/rollup.tests.config.mjs (1)
2-3: Scoped Rollup plugin imports are correct and compatible. The root package.json declares@rollup/plugin-commonjsv29.0.0 and@rollup/plugin-node-resolvev16.0.3, both of which are fully compatible with Rollup 4.53.3 and support default exports as used in these imports. The config correctly setsoutput.exports: 'named'and plugin options are appropriately configured for the test bundle.libs/dexie-react-hooks/package.json (1)
20-21: Toolchain configuration is compatible. The.mjsRollup config change, version bumps (Rollup 4.53.3, @rollup/plugin-typescript 12.3.0, TypeScript 5.3.3, React 19.2.3), and Node.js 18.0.0+ requirement are all properly aligned. No compatibility issues or breaking changes detected.addons/Dexie.Syncable/package.json (1)
58-60: Script updates to .mjs configs are correct; no QUnit mismatch concern applies.The rollup config references to
.mjsfiles are valid. However, the QUnit concern is unfounded: the codebase intentionally uses only qunitjs 1.23.1 at runtime (manually loaded in karma.common.js), while qunit 2.10.0 in devDependencies is unused. Test files import from 'QUnit' during build, which rollup marks as external. No mixed version loading occurs.addons/Dexie.Observable/package.json (2)
58-60:--skipLibCheckin build: ensure there’s still a strict typings gate somewhere
Masking lib typing errors is sometimes necessary, but I’d want a separate CI step that runstscwithout--skipLibCheck(or an equivalent) to avoid silently shipping broken.d.ts.
75-76: Test build now also uses--skipLibCheck: consider limiting this to the minimum scope
If the goal is only to avoid third-party lib type noise, OK—just make sure it’s not hiding type errors in your own test sources (especially when tests compile from JS with--allowJs).tools/build-configs/rollup.tests.config.mjs (1)
1-39: Rollup test config looks solid—please verifyexternal/globalsIDs match runtime (QUnit)
I’m assuming Karma injects a global namedQUnitand your sources import/expect that exact external ID; worth a quick sanity run to avoid “missing global” runtime failures.addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs (1)
1-33: LGTM: config matches the standardized ERRORS_TO_IGNORE/onwarn pattern used elsewhere.Small check: ensure
@rollup/plugin-node-resolvein your lockfile supportsdefaultimport here (vs{ nodeResolve }) to avoid runtime config-load failures.tools/build-configs/rollup.umd.config.mjs (1)
1-42: LGTM: consistent.mjspattern (import.meta.url-derived__dirname, banner read, plugins, onwarn).Please confirm
@rollup/plugin-node-resolvesupportsdefaultimport in your environment/version (otherwise switch toimport { nodeResolve } ...).addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.mjs (1)
1-32: LGTM: consistent with the other test Rollup configs (externals/globals/onwarn).Confirm
@rollup/plugin-node-resolvesupportsdefaultimport here (vs{ nodeResolve }) for the installed plugin version.tools/build-configs/rollup.config.mjs (1)
1-40: LGTM: consistent modern Rollup.mjsconfig (paths, banner, sourcemaps, onwarn).Please verify the correct import form for
@rollup/plugin-node-resolveat the version you’re using (default import vs named{ nodeResolve }).addons/dexie-cloud/package.json (1)
55-73: > Likely an incorrect or invalid review comment.addons/Dexie.Syncable/tools/build-configs/rollup.tests.config.js (1)
1-32: Remove this unused.jsRollup config—the.mjsversion is the active one.All build scripts explicitly invoke
rollup.tests.config.mjs, not the.jsvariant. The.jsfile is dead code and unnecessarily complicates maintenance, especially since it contains ESM syntax that would fail if loaded as CommonJS without"type": "module"in package.json.tools/build-configs/rollup.modern.config.js (1)
1-36: This file is not used in the build process and can be safely removed or marked as deprecated.The active build configuration is
rollup.modern.config.mjs. Grepping the codebase shows no references torollup.modern.config.js, and the build script explicitly specifies the.mjsversion. The.jsfile appears to be legacy code that was not fully cleaned up during migration to.mjsconfigs.If keeping this file for CJS compatibility, the current mixing of ESM imports with
require()and__dirnameis actually valid in Node.js CJS contexts and will work. However, the simpler solution is to remove this unused duplicate and maintain only the.mjsversion.Likely an incorrect or invalid review comment.
addons/Dexie.Observable/tools/build-configs/rollup.tests.unit.config.js (1)
1-33: Remove this file—it is unused and has been superseded byrollup.tests.config.mjs.The just-build configuration uses
rollup.tests.config.mjsto generate test bundles. Therollup.tests.unit.config.jsandrollup.tests.unit.config.mjsfiles are not referenced anywhere in the codebase and can be deleted as dead code.Likely an incorrect or invalid review comment.
package.json (2)
1-31: File structure and exports are consistent.The package exports (
.entry with production/development conditions) and typings path remain well-formed. No issues detected in overall JSON structure.
81-83: Verify that all Rollup config .mjs files exist and are properly configured.The build scripts reference
rollup.config.mjs,rollup.umd.config.mjs,rollup.modern.config.mjs, androllup.tests.config.mjsintools/build-configs/. Ensure they:
- Exist in
tools/build-configs/directory- Use proper ES module syntax (import/export, not CommonJS require)
- Properly import the newly-added scoped Rollup plugins:
@rollup/plugin-commonjsand@rollup/plugin-node-resolveApplies to: 115-115
| // TypeScript plugin for modern build (generates .d.ts files) | ||
| const modernTypescriptPlugin = typescript({ | ||
| tsconfig: 'src/tsconfig.json', | ||
| compilerOptions: { | ||
| target: 'es2016', | ||
| declaration: true, | ||
| declarationDir: 'dist/modern', | ||
| }, | ||
| inlineSources: true, | ||
| }); | ||
|
|
||
| // TypeScript plugin for UMD build (no declarations needed) | ||
| const umdTypescriptPlugin = typescript({ | ||
| tsconfig: 'src/tsconfig.json', | ||
| compilerOptions: { | ||
| target: 'es2016', | ||
| declaration: false, | ||
| }, | ||
| inlineSources: true, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Modern build TS target is forced to es2016—suggest relying on tsconfig or bumping modern to ES2020
For a “modern” output, forcing target: 'es2016' looks unintended (and diverges from repo-wide ES2020 moves). If you need different targets for modern vs UMD, I’d remove the override for modern (let src/tsconfig.json decide) and keep UMD constrained if needed.
const modernTypescriptPlugin = typescript({
tsconfig: 'src/tsconfig.json',
compilerOptions: {
- target: 'es2016',
declaration: true,
declarationDir: 'dist/modern',
},
inlineSources: true,
});
const umdTypescriptPlugin = typescript({
tsconfig: 'src/tsconfig.json',
compilerOptions: {
target: 'es2016',
declaration: false,
},
inlineSources: true,
});Also applies to: 110-112, 151-153
🤖 Prompt for AI Agents
In addons/dexie-cloud/tools/build-configs/rollup.config.mjs around lines 32-52
(and similarly at 110-112 and 151-153), the modern TypeScript plugin is forcing
target: 'es2016' which conflicts with the repo's ES2020 direction; remove the
compilerOptions.target override for the modernTypescriptPlugin so it inherits
the target from src/tsconfig.json (or change it to 'es2020' if you explicitly
want a modern target), while keeping or explicitly setting the UMD plugin's
target to the lower ES version if UMD needs to remain constrained; update any
duplicated blocks accordingly to keep modern builds consistent with tsconfig.
| "qunit": "2.10.0", | ||
| "qunitjs": "1.23.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable version of qunit npm package
💡 Result:
The latest stable qunit npm package version is 2.24.3 (released November 2025). [1][2]
Sources:
[1] https://qunitjs.com/ (Current Release: v2.24.3).
[2] QUnit Blog — "QUnit 2.24.3 Released: Bug fix" (November 2025).
🌐 Web query:
qunitjs npm package deprecated maintained status
💡 Result:
Short answer: The npm package qunitjs is deprecated — its last published version is 2.4.1 and the package page states future updates are published under the package name qunit. Use the qunit package (active, recent releases). [1][2]
Sources:
- qunitjs npm (deprecated notice, last publish 8 years ago). [1]
- qunit npm (active package, recent releases). [2]
Remove deprecated qunitjs dependency; update qunit to current version.
The qunitjs package is deprecated (last published 8 years ago; no longer maintained). Migrate to the active qunit package instead. Additionally, qunit 2.10.0 (2019) is severely outdated—current stable version is 2.24.3 (November 2025). Remove the qunitjs dependency and update qunit to a recent 2.x version.
🤖 Prompt for AI Agents
In addons/dexie-export-import/package.json around lines 43-44, remove the
deprecated "qunitjs" entry and update the "qunit" dependency to a current 2.x
release (e.g. "2.24.3" or "^2.24.3"); after editing package.json, run your
package manager (npm/yarn) to update the lockfile and reinstall, then run the
test suite and fix any API-breaking test changes introduced by the newer qunit
version.
| "qunit": "2.10.0", | ||
| "qunitjs": "1.23.1", | ||
| "typescript": "^5.3.3", | ||
| "uglify-js": "^3.5.6" | ||
| "uglify-js": "^3.5.6", | ||
| "undici-types": "^6.21.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the Karma configuration
fd -t f "karma" --max-one-result
fd -t f "karma.conf"
find . -name "karma.conf.*" -o -name ".karma*"Repository: dexie/Dexie.js
Length of output: 1228
🏁 Script executed:
# Look for test setup and configuration files
fd -t f "test" . | head -20
find . -path ./node_modules -prune -o -name "*test*" -type f | grep -v node_modules | head -20Repository: dexie/Dexie.js
Length of output: 2839
🏁 Script executed:
# Check for rollup configuration
fd -t f "rollup"
find . -name "rollup.config.*" -o -name ".rolluprc*"Repository: dexie/Dexie.js
Length of output: 1768
🏁 Script executed:
# Look at package.json structure and scripts
cat addons/Dexie.Observable/package.json | head -100Repository: dexie/Dexie.js
Length of output: 3747
🏁 Script executed:
# Check the Karma configuration for the unit tests
cat addons/Dexie.Observable/test/unit/karma.conf.jsRepository: dexie/Dexie.js
Length of output: 1108
🏁 Script executed:
# Check the rollup test config to see dependencies
cat addons/Dexie.Observable/tools/build-configs/rollup.tests.config.mjs | head -50Repository: dexie/Dexie.js
Length of output: 999
🏁 Script executed:
# Search for imports/requires of qunit or qunitjs in the test files
rg "require.*qunit|import.*qunit" addons/Dexie.Observable/test/ --type js --type tsRepository: dexie/Dexie.js
Length of output: 40
🏁 Script executed:
# Check the common karma configuration to see how QUnit is set up
cat test/karma.common.js | head -100Repository: dexie/Dexie.js
Length of output: 2620
🏁 Script executed:
# Look for any qunit references in karma common or main configs
rg "qunit" test/karma.common.js test/karma.conf.js -iRepository: dexie/Dexie.js
Length of output: 578
🏁 Script executed:
# Check npm to understand what qunit vs qunitjs packages are
# Search node_modules or package-lock info to see versions and purposes
fd "package.json" node_modules/qunit node_modules/qunitjs 2>/dev/null | head -5Repository: dexie/Dexie.js
Length of output: 40
🏁 Script executed:
# Search for any usage of the qunit package (not qunitjs) in the codebase
rg "node_modules/qunit/" . --type js --type ts --type json
rg "from.*qunit['\"]|require.*qunit['\"]" . --type js --type ts -g "!node_modules"Repository: dexie/Dexie.js
Length of output: 1627
🏁 Script executed:
# Check if qunit 2.x is referenced anywhere in build configs or test setups
rg "qunit['\"]" addons/Dexie.Observable/ --type js --type json -C 2Repository: dexie/Dexie.js
Length of output: 1715
🏁 Script executed:
# Check build scripts and just-build configuration for qunit references
cat addons/Dexie.Observable/package.json | grep -A 30 "just-build"Repository: dexie/Dexie.js
Length of output: 2730
🏁 Script executed:
# Search for qunit in build-related files
rg "qunit" addons/Dexie.Observable/tools/ --type js --type mjs -C 2Repository: dexie/Dexie.js
Length of output: 85
🏁 Script executed:
# Check if there are any scripts that directly use the qunit CLI
rg "qunit\s" addons/Dexie.Observable/ --type json --type js -C 1Repository: dexie/Dexie.js
Length of output: 545
Remove unused qunit 2.10.0 dependency
The karma setup explicitly uses qunitjs 1.23.1 and loads it manually via node_modules/qunitjs/qunit/qunit.js (see test/karma.common.js). The comment there states: "Use qunitjs 1.23.1 instead of qunit 2.x" and "We load it manually in files array instead of using the 'qunit' framework." The qunit 2.10.0 package is not referenced in any build scripts, karma configuration, or test files for Dexie.Observable, making it an unused dependency. Remove it from devDependencies to avoid confusion about which QUnit version is active.
🤖 Prompt for AI Agents
In addons/Dexie.Observable/package.json around lines 91 to 95, the devDependency
"qunit": "2.10.0" is unused and conflicts with the intended qunitjs 1.23.1
setup; remove the "qunit" entry from devDependencies so only "qunitjs": "1.23.1"
remains, then run a quick npm/yarn install and verify tests/karma.common.js
still load node_modules/qunitjs/qunit/qunit.js correctly.
| import replace from '@rollup/plugin-replace'; | ||
| // @ts-ignore: requires tsconfig settings that we don't need for the web build but is ok here in the build config. | ||
| import pkg from '../../package.json' assert { type: 'json' }; | ||
| import pkg from '../../package.json' with { type: 'json' }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JSON import attributes "with { type: 'json' }" Node.js 18 20 22 support compatibility
💡 Result:
Short answer
- import ... with { type: 'json' } (the "import attributes" / with syntax) is supported in Node.js starting with v18.20.0 and in later v20+ / v22 releases. [1][2]
- Older Node releases (Node 16/17 and early 18/20 builds) used the earlier import-assertions syntax: import ... assert { type: 'json' }. The assert form works on Node 17.1.0+ (and some 16.x experimental builds) but was replaced by with; switching to with will break very old Node versions. [2][3]
Practical compatibility guidance
- Node 18: only versions >= 18.20.0 support with { type: 'json' }; earlier 18.x accept assert instead. [1]
- Node 20: recent 20.x releases include support for with (the change landed in the 20 line around 20.10.0); older 20.x builds before that may still use assert. [1][2]
- Node 22: expects the with syntax (import assertions were removed/replaced). Use with { type: 'json' }. [1][3]
Sources
- Node.js ESM docs (history / import attributes; notes that switch from assert → with happened in v18.20.0 and later, and that type: 'json' is required). [1][2]
- Community notes / troubleshooting (explanation of assert → with change and compatibility trade-offs). [3]
If you want, I can check specific patch versions you care about (for example: 18.16, 18.20, 20.5, 20.15, 22.x) and confirm whether they accept the with syntax.
🏁 Script executed:
# Check package.json for Node version requirements
fd -e package.json -x cat -n {} \; | head -50Repository: dexie/Dexie.js
Length of output: 40
🏁 Script executed:
# Search for readFileSync + JSON.parse pattern usage in the codebase
rg "readFileSync.*JSON\.parse|JSON\.parse.*readFileSync" -A 2 -B 2Repository: dexie/Dexie.js
Length of output: 3070
🏁 Script executed:
# Check the specific file and surrounding context
cat -n addons/y-dexie/tools/build-configs/rollup.test.unit.config.js | head -20Repository: dexie/Dexie.js
Length of output: 821
🏁 Script executed:
# Check Node version requirements in main package.json
cat -n package.json | grep -A 5 -B 5 "engines\|node"Repository: dexie/Dexie.js
Length of output: 2668
🏁 Script executed:
# Check if there's any documentation or comments about why this file uses import attributes
cat -n addons/y-dexie/tools/build-configs/rollup.test.unit.config.jsRepository: dexie/Dexie.js
Length of output: 1672
🏁 Script executed:
# List all rollup config files to confirm the inconsistency
find . -path ./node_modules -prune -o -name "rollup*.config.*" -type f -printRepository: dexie/Dexie.js
Length of output: 1385
🏁 Script executed:
# Check the dexie-cloud test unit config to see if it also uses import attributes
cat -n addons/dexie-cloud/tools/build-configs/rollup.test.unit.config.js | head -20Repository: dexie/Dexie.js
Length of output: 822
🏁 Script executed:
# Look for .nvmrc, .npmrc or other version hints
find . -maxdepth 2 -name ".nvmrc" -o -name ".node-version" -o -name ".npmrc" | xargs cat 2>/dev/nullRepository: dexie/Dexie.js
Length of output: 81
Inconsistent JSON import pattern—align with rest of codebase.
This file uses import pkg from '../../package.json' with { type: 'json' }, which differs from all production rollup configs that use readFileSync(path.resolve(...)) + JSON.parse(). While with { type: 'json' } is supported in Node 18.20.0+, 20.10.0+, and 22.x, adopting the established readFileSync pattern would improve consistency across all build configs and eliminate any version compatibility concerns.
🤖 Prompt for AI Agents
In addons/y-dexie/tools/build-configs/rollup.test.unit.config.js around line 7,
replace the JSON module import "import pkg from '../../package.json' with {
type: 'json' }" with the project's established pattern: require fs and path, use
path.resolve to build the absolute path to ../../package.json, read it with
fs.readFileSync(..., 'utf8') and parse it with JSON.parse(...); assign the
parsed object to pkg so the file matches other rollup configs and avoids Node
JSON import version constraints.
| // QUnit 1.x exports asyncTest, test, etc. as global functions | ||
| // but rollup expects them as properties of the QUnit object when using named imports | ||
| // Make them accessible both ways | ||
| if (typeof asyncTest !== 'undefined' && !QUnit.asyncTest) { | ||
| QUnit.asyncTest = asyncTest; | ||
| } | ||
| if (typeof test !== 'undefined' && !QUnit.test) { | ||
| QUnit.test = test; | ||
| } | ||
| if (typeof module !== 'undefined' && !QUnit.module) { | ||
| QUnit.module = module; | ||
| } | ||
| if (typeof ok !== 'undefined' && !QUnit.ok) { | ||
| QUnit.ok = ok; | ||
| } | ||
| if (typeof equal !== 'undefined' && !QUnit.equal) { | ||
| QUnit.equal = equal; | ||
| } | ||
| if (typeof notEqual !== 'undefined' && !QUnit.notEqual) { | ||
| QUnit.notEqual = notEqual; | ||
| } | ||
| if (typeof deepEqual !== 'undefined' && !QUnit.deepEqual) { | ||
| QUnit.deepEqual = deepEqual; | ||
| } | ||
| if (typeof notDeepEqual !== 'undefined' && !QUnit.notDeepEqual) { | ||
| QUnit.notDeepEqual = notDeepEqual; | ||
| } | ||
| if (typeof strictEqual !== 'undefined' && !QUnit.strictEqual) { | ||
| QUnit.strictEqual = strictEqual; | ||
| } | ||
| if (typeof notStrictEqual !== 'undefined' && !QUnit.notStrictEqual) { | ||
| QUnit.notStrictEqual = notStrictEqual; | ||
| } | ||
| if (typeof start !== 'undefined' && !QUnit.start) { | ||
| QUnit.start = start; | ||
| } | ||
| if (typeof stop !== 'undefined' && !QUnit.stop) { | ||
| QUnit.stop = stop; | ||
| } |
There was a problem hiding this comment.
Guard module/helpers by function type (current code can break in bundler envs)
module is a particularly risky global name—many environments define it as an object. Please only alias when the global is actually the expected function.
- if (typeof asyncTest !== 'undefined' && !QUnit.asyncTest) {
+ if (typeof asyncTest === 'function' && !QUnit.asyncTest) {
QUnit.asyncTest = asyncTest;
}
- if (typeof test !== 'undefined' && !QUnit.test) {
+ if (typeof test === 'function' && !QUnit.test) {
QUnit.test = test;
}
- if (typeof module !== 'undefined' && !QUnit.module) {
+ if (typeof module === 'function' && !QUnit.module) {
QUnit.module = module;
}
- if (typeof ok !== 'undefined' && !QUnit.ok) {
+ if (typeof ok === 'function' && !QUnit.ok) {
QUnit.ok = ok;
}
- if (typeof equal !== 'undefined' && !QUnit.equal) {
+ if (typeof equal === 'function' && !QUnit.equal) {
QUnit.equal = equal;
}
- if (typeof notEqual !== 'undefined' && !QUnit.notEqual) {
+ if (typeof notEqual === 'function' && !QUnit.notEqual) {
QUnit.notEqual = notEqual;
}
- if (typeof deepEqual !== 'undefined' && !QUnit.deepEqual) {
+ if (typeof deepEqual === 'function' && !QUnit.deepEqual) {
QUnit.deepEqual = deepEqual;
}
- if (typeof notDeepEqual !== 'undefined' && !QUnit.notDeepEqual) {
+ if (typeof notDeepEqual === 'function' && !QUnit.notDeepEqual) {
QUnit.notDeepEqual = notDeepEqual;
}
- if (typeof strictEqual !== 'undefined' && !QUnit.strictEqual) {
+ if (typeof strictEqual === 'function' && !QUnit.strictEqual) {
QUnit.strictEqual = strictEqual;
}
- if (typeof notStrictEqual !== 'undefined' && !QUnit.notStrictEqual) {
+ if (typeof notStrictEqual === 'function' && !QUnit.notStrictEqual) {
QUnit.notStrictEqual = notStrictEqual;
}
- if (typeof start !== 'undefined' && !QUnit.start) {
+ if (typeof start === 'function' && !QUnit.start) {
QUnit.start = start;
}
- if (typeof stop !== 'undefined' && !QUnit.stop) {
+ if (typeof stop === 'function' && !QUnit.stop) {
QUnit.stop = stop;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // QUnit 1.x exports asyncTest, test, etc. as global functions | |
| // but rollup expects them as properties of the QUnit object when using named imports | |
| // Make them accessible both ways | |
| if (typeof asyncTest !== 'undefined' && !QUnit.asyncTest) { | |
| QUnit.asyncTest = asyncTest; | |
| } | |
| if (typeof test !== 'undefined' && !QUnit.test) { | |
| QUnit.test = test; | |
| } | |
| if (typeof module !== 'undefined' && !QUnit.module) { | |
| QUnit.module = module; | |
| } | |
| if (typeof ok !== 'undefined' && !QUnit.ok) { | |
| QUnit.ok = ok; | |
| } | |
| if (typeof equal !== 'undefined' && !QUnit.equal) { | |
| QUnit.equal = equal; | |
| } | |
| if (typeof notEqual !== 'undefined' && !QUnit.notEqual) { | |
| QUnit.notEqual = notEqual; | |
| } | |
| if (typeof deepEqual !== 'undefined' && !QUnit.deepEqual) { | |
| QUnit.deepEqual = deepEqual; | |
| } | |
| if (typeof notDeepEqual !== 'undefined' && !QUnit.notDeepEqual) { | |
| QUnit.notDeepEqual = notDeepEqual; | |
| } | |
| if (typeof strictEqual !== 'undefined' && !QUnit.strictEqual) { | |
| QUnit.strictEqual = strictEqual; | |
| } | |
| if (typeof notStrictEqual !== 'undefined' && !QUnit.notStrictEqual) { | |
| QUnit.notStrictEqual = notStrictEqual; | |
| } | |
| if (typeof start !== 'undefined' && !QUnit.start) { | |
| QUnit.start = start; | |
| } | |
| if (typeof stop !== 'undefined' && !QUnit.stop) { | |
| QUnit.stop = stop; | |
| } | |
| // QUnit 1.x exports asyncTest, test, etc. as global functions | |
| // but rollup expects them as properties of the QUnit object when using named imports | |
| // Make them accessible both ways | |
| if (typeof asyncTest === 'function' && !QUnit.asyncTest) { | |
| QUnit.asyncTest = asyncTest; | |
| } | |
| if (typeof test === 'function' && !QUnit.test) { | |
| QUnit.test = test; | |
| } | |
| if (typeof module === 'function' && !QUnit.module) { | |
| QUnit.module = module; | |
| } | |
| if (typeof ok === 'function' && !QUnit.ok) { | |
| QUnit.ok = ok; | |
| } | |
| if (typeof equal === 'function' && !QUnit.equal) { | |
| QUnit.equal = equal; | |
| } | |
| if (typeof notEqual === 'function' && !QUnit.notEqual) { | |
| QUnit.notEqual = notEqual; | |
| } | |
| if (typeof deepEqual === 'function' && !QUnit.deepEqual) { | |
| QUnit.deepEqual = deepEqual; | |
| } | |
| if (typeof notDeepEqual === 'function' && !QUnit.notDeepEqual) { | |
| QUnit.notDeepEqual = notDeepEqual; | |
| } | |
| if (typeof strictEqual === 'function' && !QUnit.strictEqual) { | |
| QUnit.strictEqual = strictEqual; | |
| } | |
| if (typeof notStrictEqual === 'function' && !QUnit.notStrictEqual) { | |
| QUnit.notStrictEqual = notStrictEqual; | |
| } | |
| if (typeof start === 'function' && !QUnit.start) { | |
| QUnit.start = start; | |
| } | |
| if (typeof stop === 'function' && !QUnit.stop) { | |
| QUnit.stop = stop; | |
| } |
🤖 Prompt for AI Agents
In test/karma-env.js around lines 5 to 43, some globals (notably module) are
only checked for !== 'undefined' which can mistakenly alias non-function globals
from bundler environments; change the guards to ensure the global is a function
before aliasing (use typeof <name> === 'function' && !QUnit.<name>) for module
and any other helpers that are expected to be functions so you only assign
QUnit.<name> when the global is actually a function.
| let content = fs.readFileSync(filePath, 'utf-8'); | ||
| const lines = content.split('\n'); | ||
|
|
||
| // Find all "export declare module Dexie {" blocks | ||
| let moduleStarts = []; | ||
| let moduleEnds = []; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| if (lines[i].trim().startsWith('export declare module Dexie {')) { | ||
| moduleStarts.push(i); | ||
| } | ||
| } | ||
|
|
||
| // Find the end of each module (closing brace) | ||
| for (let start of moduleStarts) { | ||
| let braceCount = 0; | ||
| for (let i = start; i < lines.length; i++) { | ||
| const line = lines[i].trim(); | ||
| if (line.includes('{')) braceCount += (line.match(/\{/g) || []).length; | ||
| if (line.includes('}')) braceCount -= (line.match(/\}/g) || []).length; | ||
| if (braceCount === 0 && i > start) { | ||
| moduleEnds.push(i); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If we have duplicates, remove all but the first | ||
| if (moduleStarts.length > 1) { | ||
| console.log(`Found ${moduleStarts.length} duplicate module declarations. Removing duplicates...`); | ||
|
|
||
| // Remove duplicates in reverse order to preserve line numbers | ||
| for (let i = moduleStarts.length - 1; i >= 1; i--) { | ||
| const start = moduleStarts[i]; | ||
| // Find the start of the comment block before this duplicate | ||
| let commentStart = start; | ||
| while (commentStart > 0 && (lines[commentStart - 1].trim().startsWith('//') || lines[commentStart - 1].trim() === '')) { | ||
| commentStart--; | ||
| } | ||
|
|
||
| const end = moduleEnds[i]; | ||
| lines.splice(commentStart, end - commentStart + 1); | ||
| } | ||
|
|
||
| content = lines.join('\n'); | ||
| fs.writeFileSync(filePath, content, 'utf-8'); | ||
| console.log('Fixed duplicate module declarations'); | ||
| } else { | ||
| console.log('No duplicate module declarations found'); | ||
| } |
There was a problem hiding this comment.
Add safety guards (unmatched braces / comment trimming) to avoid corrupting the .d.ts.
Today, if a module end isn’t found (or brace counting is thrown off), moduleEnds[i] can be undefined and the splice will behave unexpectedly. Also, walking upward over blank lines can eat unrelated comment blocks.
Suggested patch:
let content = fs.readFileSync(filePath, 'utf-8');
-const lines = content.split('\n');
+const eol = content.includes('\r\n') ? '\r\n' : '\n';
+const lines = content.split(/\r?\n/);
// Find all "export declare module Dexie {" blocks
let moduleStarts = [];
let moduleEnds = [];
@@
// Find the end of each module (closing brace)
for (let start of moduleStarts) {
let braceCount = 0;
for (let i = start; i < lines.length; i++) {
const line = lines[i].trim();
if (line.includes('{')) braceCount += (line.match(/\{/g) || []).length;
if (line.includes('}')) braceCount -= (line.match(/\}/g) || []).length;
if (braceCount === 0 && i > start) {
moduleEnds.push(i);
break;
}
}
+ if (moduleEnds.length !== moduleStarts.indexOf(start) + 1) {
+ console.error(`Could not find end brace for module starting at line ${start + 1}. Aborting.`);
+ process.exit(2);
+ }
}
@@
for (let i = moduleStarts.length - 1; i >= 1; i--) {
const start = moduleStarts[i];
// Find the start of the comment block before this duplicate
let commentStart = start;
- while (commentStart > 0 && (lines[commentStart - 1].trim().startsWith('//') || lines[commentStart - 1].trim() === '')) {
+ while (commentStart > 0 && lines[commentStart - 1].trim().startsWith('//')) {
commentStart--;
}
@@
- content = lines.join('\n');
+ content = lines.join(eol);
fs.writeFileSync(filePath, content, 'utf-8');(If you want, I can suggest a slightly more robust “find matching brace” routine that ignores braces inside // comments.)
Summary by CodeRabbit
New Features
Dependencies & Infrastructure
Chores
Build
✏️ Tip: You can customize this high-level summary in your review settings.