Port custom DisposableStack polyfills and simplify build scripts#63
Port custom DisposableStack polyfills and simplify build scripts#63
Conversation
Source upstream commit: 7e0d6897615c493e1154a5670f32a1686f9ec1ce Port scope: adapted (keeps fork dependency versions, adds runtime-compatible test assertions)
Source upstream commit: d6e35b1ac58f6665071ca0633a1444838123c0ed Port scope: adapted (keeps fork script layout, applies build-script simplification and root clean:ts gate)
…ps/cli, and @react-navigation packages.
…bplist-parser` 0.3.1 to bun.lock.
📝 WalkthroughWalkthroughPřechod mnoha skriptů z Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP as HTTP Server
participant Relay as Relay Upgrade Handler
participant WS as WebSocketServer
participant Shutdown as Shutdown Manager
Client->>HTTP: HTTP Upgrade request (ownerId)
HTTP->>Relay: forward upgrade
Relay->>Relay: authorize ownerId (try/catch)
alt auth ok
Relay->>WS: completeUpgrade (establish WS)
WS-->>Client: WebSocket open
else auth fail
Relay->>HTTP: rejectUpgrade(400/"Bad Request")
HTTP-->>Client: HTTP error + close socket
end
Note over Shutdown,WS: On shutdown
Shutdown->>WS: close all clients (with timeout/terminate)
Shutdown->>HTTP: close server (with timeout)
WS-->>Shutdown: closed
HTTP-->>Shutdown: closed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 239bb3c0a1
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR modernizes the monorepo’s build/clean tooling (moving off rimraf), ports/owns DisposableStack + AsyncDisposableStack polyfills for cross-runtime determinism (incl. WebKit), and improves test/coverage lane reliability across Bun/Node/browser environments.
Changes:
- Replaced
rimrafusage across packages/examples with a Bun-run TypeScript removal script (scripts/rm.mts) and added a rootclean:tsstep to clean TypeScript build artifacts beforeturbo build. - Introduced/expanded owned polyfills (
installPolyfills) for resource-management APIs and updated test setups (Node + browser) to install them explicitly. - Improved runtime compatibility and test stability (Relay upgrade rejection handling, close timeouts, 127.0.0.1 binding, Bun-based tree-shaking lane, and a dedicated Istanbul coverage config).
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.coverage.config.mts | Adds a coverage-specific Vitest workspace config (Istanbul provider, excludes browser lanes). |
| scripts/rm.mts | New Bun-run deletion helper replacing rimraf in scripts. |
| packages/web/vitest.config.ts | Ensures polyfills are installed for browser tests via setupFiles. |
| packages/web/test/_browserSetup.ts | Installs @evolu/common polyfills before browser tests. |
| packages/web/package.json | Replaces rimraf in clean script with rm.mts. |
| packages/vue/package.json | Replaces rimraf in clean script with rm.mts. |
| packages/tauri/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/tanstack-start/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/svelte/package.json | Replaces rimraf; bumps Svelte; aligns peer dependency floor. |
| packages/react/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/react-web/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/react-native/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/nodejs/test/setup.ts | Installs polyfills in Node test setup. |
| packages/nodejs/test/Sqlite.test.ts | Uses env override to force Bun/Node sqlite runtime paths in tests. |
| packages/nodejs/test/Relay.test.ts | Stabilizes websocket error handling + loosens runtime-specific error expectations. |
| packages/nodejs/src/local-first/Relay.ts | Adds upgrade rejection helper, hardens auth/upgrade error handling, and improves shutdown/listen reliability. |
| packages/nodejs/src/Sqlite.ts | Adds EVOLU_NODEJS_SQLITE_RUNTIME override and centralizes Bun-runtime detection. |
| packages/nodejs/package.json | Replaces rimraf in clean script with rm.mts. |
| packages/common/test/local-first/Sync.test.ts | Adjusts quota test to re-encrypt change using updated timestamp value. |
| packages/common/test/_globalSetup.ts | Binds test server to 127.0.0.1 and adds close timeouts/termination to avoid hangs. |
| packages/common/test/_browserSetup.ts | Switches to installPolyfills() instead of upstream disposablestack/auto. |
| packages/common/test/WebSocket.test.ts | Uses 127.0.0.1 and conditionally skips server-dependent cases when not applicable. |
| packages/common/test/TreeShaking.test.ts | Replaces webpack-based bundling with Bun bundler for tree-shaking lane tests. |
| packages/common/test/Polyfills.test.ts | Adds extensive conformance tests for owned DisposableStack polyfills. |
| packages/common/src/local-first/Protocol.ts | Makes stack trace capture safe when Error.captureStackTrace is unavailable. |
| packages/common/src/Type.ts | Uses nodeBuffer abstraction for base64url encode/decode to avoid assuming global Buffer. |
| packages/common/src/Task.ts | Improves AsyncDisposableStack handling of sync disposables + uses global setImmediate fallback. |
| packages/common/src/Polyfills.ts | Replaces runtime dependency on disposablestack with owned polyfill implementation. |
| packages/common/src/Platform.ts | Adds nodeBuffer/NodeBufferLike and updates hasNodeBuffer detection. |
| packages/common/src/Buffer.ts | Makes stack trace capture safe when Error.captureStackTrace is unavailable. |
| packages/common/package.json | Removes disposablestack/webpack deps; replaces rimraf usage with rm.mts. |
| packages/bun/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| packages/astro/package.json | Replaces rimraf in clean script; build now cleans dist via rm.mts. |
| package.json | Adds clean:ts, wires it into root build, switches coverage config, removes rimraf/webpack deps, bumps turbo, adds Istanbul coverage plugin. |
| examples/vue-vite-pwa/package.json | Replaces rimraf in clean script with rm.mts. |
| examples/tauri/package.json | Replaces rimraf in clean script; bumps @tauri-apps/cli. |
| examples/svelte-vite-pwa/package.json | Replaces rimraf in clean script; bumps Svelte. |
| examples/react-vite-pwa/package.json | Replaces rimraf in clean script. |
| examples/react-nextjs/package.json | Replaces rimraf in clean script. |
| examples/react-expo/package.json | Replaces rimraf in clean script; bumps react-native-quick-crypto. |
| examples/react-electron/package.json | Replaces rimraf in clean script. |
| bun.lock | Lockfile updates reflecting dependency removals/bumps and new coverage plugin. |
| apps/relay/tsconfig.json | Explicitly includes Node types/typeRoots for better type checking. |
| apps/relay/package.json | Switches build to tsc --build; replaces rimraf in clean script with rm.mts. |
| if (targets.length === 0) { | ||
| console.error("Usage: bun ./scripts/rm.mts <path> [<path> ...]"); | ||
| process.exit(1); |
There was a problem hiding this comment.
The usage message hard-codes bun ./scripts/rm.mts ..., but this script is also invoked from package/example directories (e.g. bun ../../scripts/rm.mts ...). Consider printing a generic usage string (or using process.argv[1]) so the guidance is accurate regardless of the current working directory.
![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
🧹 Nitpick comments (4)
scripts/rm.mts (1)
10-19: Zvážit explicitní ošetření chyb pro čitelnější výstup.Skript je funkční, ale pokud některé mazání selže,
Promise.allvyhodí neošetřenou výjimku. Pro lepší diagnostiku by stálo za zvážení wrappnout do try-catch s informativnější chybovou zprávou.♻️ Volitelné vylepšení error handlingu
-await Promise.all( - targets.map((target) => - rm(target, { - force: true, - recursive: true, - maxRetries: 3, - retryDelay: 100, - }), - ), -); +try { + await Promise.all( + targets.map((target) => + rm(target, { + force: true, + recursive: true, + maxRetries: 3, + retryDelay: 100, + }), + ), + ); +} catch (error) { + console.error("Failed to remove targets:", error); + process.exit(1); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/rm.mts` around lines 10 - 19, Wrap the Promise.all call that runs targets.map and rm(...) in a try-catch to explicitly handle failures: catch errors from the overall Promise.all and produce an informative error message (including which target(s) failed and the original error) or aggregate per-target results so you can log successes and failures; reference the Promise.all invocation, targets.map, and the rm function to locate the code and replace the unhandled rejection with a try-catch that logs a clear diagnostic and rethrows or exits as appropriate.apps/relay/package.json (1)
8-8: Build bez předchozího úklidu může nechat zastaralé artefakty vdist.Na Line 8 se teď build nespouští nad čistým výstupem; při odstranění/renamu souborů mohou ve
distzůstat staré JS soubory, které pak načtestartna Line 9.💡 Návrh úpravy
- "build": "tsc --build tsconfig.json", + "build": "bun ../../scripts/rm.mts dist && tsc --build tsconfig.json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/relay/package.json` at line 8, The current package.json "build" script runs tsc without cleaning dist first, which can leave stale JS artifacts that "start" later serves; update package.json scripts by adding a "clean" script (e.g., using rimraf or rm -rf to remove dist) and change the "build" script to run the clean step first (e.g., "npm run clean && tsc --build tsconfig.json") so the "build" and subsequent "start" scripts always operate on a fresh dist directory.packages/common/test/_globalSetup.ts (1)
15-30:closeWithTimeoutby měl čistit timer po úspěšném uzavření.Na Line 28 se timer vždy naplánuje, ale při rychlém
closecallbacku na Line 27 se neruší. V testech to zbytečně kumuluje pending callbacky.💡 Návrh úpravy
const closeWithTimeout = ( close: (callback: () => void) => void, timeoutMs = 500, ): Promise<void> => new Promise<void>((resolve) => { let settled = false; + let timeout: ReturnType<typeof setTimeout> | undefined; const settle = () => { if (settled) return; settled = true; + if (timeout) clearTimeout(timeout); resolve(); }; close(settle); - const timeout = setTimeout(settle, timeoutMs); + timeout = setTimeout(settle, timeoutMs); timeout.unref?.(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/test/_globalSetup.ts` around lines 15 - 30, Funkce closeWithTimeout nechává timer běžet i když close zavolá callback dříve; oprav to tak, že uložíš návratovou hodnotu setTimeout do proměnné (např. timeout) před voláním close, a v interní funkci settle explicitně zavoláš clearTimeout(timeout) před resolve(), přičemž zachováš volání timeout.unref?.() po nastavení setTimeout; upravuj funkci closeWithTimeout tak, aby timeout byl přístupný v rámci settle a byl vždy zrušen při úspěšném uzavření.packages/nodejs/test/Relay.test.ts (1)
191-193: Rozšířené regex aserce pro cross-platform kompatibilitu.Použití regexu pro akceptování buď specifické HTTP chyby (
Unexpected server response: 400/401) nebo generické zprávy (Connection ended) zvyšuje robustnost testů napříč různými runtime prostředími.Zvažte přidání komentáře vysvětlujícího, proč jsou obě varianty akceptovatelné, aby budoucí vývojáři věděli, že to není chyba v testu:
📝 Návrh na přidání dokumentačního komentáře
+ // Runtime-dependent: ws reports HTTP status, some environments report "Connection ended" expect(String(error.message)).toMatch( /Unexpected server response: 400|Connection ended/, );Also applies to: 219-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nodejs/test/Relay.test.ts` around lines 191 - 193, Update the flaky assertion to explicitly accept both HTTP status variants and the generic runtime message: change the regex used in expect(String(error.message)).toMatch(...) to include 400 and 401 (e.g. /Unexpected server response: 400|Unexpected server response: 401|Connection ended/) and add a short inline comment above that expect explaining this dual-match is intentional for cross-platform/runtime differences; repeat the same change + comment for the other occurrence of expect(String(error.message)).toMatch at the second location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 61: The "clean" npm script currently deletes the lockfile which harms
dependency reproducibility; edit the package.json "clean" script (the value for
the "clean" key) to remove "bun.lock" from the list of paths to delete so the
lockfile is preserved during clean operations while keeping the other cleanup
targets intact.
In `@packages/common/src/Task.ts`:
- Around line 1068-1086: The register function currently treats any object with
Symbol.dispose property as disposable without verifying it's callable; update
the check in register to ensure the Symbol.dispose property is a function before
wrapping it for async disposal (i.e., change the guard to something like: if
(Symbol.dispose in value && typeof (value as any)[Symbol.dispose] ===
'function') { ... } ), and if it's not a function fall back to calling
this.#stack.use(value as T) (or optionally throw a clear error) so non-callable
dispose properties don't only fail later during cleanup; reference the register
function, the Symbol.dispose check, the Disposable type, and the this.#stack.use
/ Symbol.asyncDispose wrapper when making the change.
In `@packages/common/test/TreeShaking.test.ts`:
- Around line 234-235: The current early-return in the test (the line using
isBunRuntime and process.platform) hides a misconfigured compat lane; instead,
detect the EVOLU_TREE_SHAKING_COMPAT environment flag and if it's set but the
runtime check fails (isBunRuntime is false or platform is win32), make the test
fail/throw with a clear message mentioning EVOLU_TREE_SHAKING_COMPAT,
isBunRuntime and process.platform so CI surfaces the misconfiguration rather
than returning silently.
In `@packages/common/test/WebSocket.test.ts`:
- Around line 44-46: wsTest is currently set to test.skip when !isServer which
disables the browser-specific WebSocket tests; change the wsTest assignment so
it does not skip when running in a browser environment by detecting
browser/WebSocket availability (e.g., check typeof window !== 'undefined' or
presence of window.WebSocket) instead of only isServer, so wsTest("connects,
receives message, sends message, and disposes", ...) runs in the browser branch;
update the wsTest definition (the const wsTest = ... line) to use that
browser-aware condition.
---
Nitpick comments:
In `@apps/relay/package.json`:
- Line 8: The current package.json "build" script runs tsc without cleaning dist
first, which can leave stale JS artifacts that "start" later serves; update
package.json scripts by adding a "clean" script (e.g., using rimraf or rm -rf to
remove dist) and change the "build" script to run the clean step first (e.g.,
"npm run clean && tsc --build tsconfig.json") so the "build" and subsequent
"start" scripts always operate on a fresh dist directory.
In `@packages/common/test/_globalSetup.ts`:
- Around line 15-30: Funkce closeWithTimeout nechává timer běžet i když close
zavolá callback dříve; oprav to tak, že uložíš návratovou hodnotu setTimeout do
proměnné (např. timeout) před voláním close, a v interní funkci settle
explicitně zavoláš clearTimeout(timeout) před resolve(), přičemž zachováš volání
timeout.unref?.() po nastavení setTimeout; upravuj funkci closeWithTimeout tak,
aby timeout byl přístupný v rámci settle a byl vždy zrušen při úspěšném
uzavření.
In `@packages/nodejs/test/Relay.test.ts`:
- Around line 191-193: Update the flaky assertion to explicitly accept both HTTP
status variants and the generic runtime message: change the regex used in
expect(String(error.message)).toMatch(...) to include 400 and 401 (e.g.
/Unexpected server response: 400|Unexpected server response: 401|Connection
ended/) and add a short inline comment above that expect explaining this
dual-match is intentional for cross-platform/runtime differences; repeat the
same change + comment for the other occurrence of
expect(String(error.message)).toMatch at the second location.
In `@scripts/rm.mts`:
- Around line 10-19: Wrap the Promise.all call that runs targets.map and rm(...)
in a try-catch to explicitly handle failures: catch errors from the overall
Promise.all and produce an informative error message (including which target(s)
failed and the original error) or aggregate per-target results so you can log
successes and failures; reference the Promise.all invocation, targets.map, and
the rm function to locate the code and replace the unhandled rejection with a
try-catch that logs a clear diagnostic and rethrows or exits as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fef555db-0622-4132-bf65-f8b782cbf514
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
apps/relay/package.jsonapps/relay/tsconfig.jsonexamples/react-electron/package.jsonexamples/react-expo/package.jsonexamples/react-nextjs/package.jsonexamples/react-vite-pwa/package.jsonexamples/svelte-vite-pwa/package.jsonexamples/tauri/package.jsonexamples/vue-vite-pwa/package.jsonpackage.jsonpackages/astro/package.jsonpackages/bun/package.jsonpackages/common/package.jsonpackages/common/src/Buffer.tspackages/common/src/Platform.tspackages/common/src/Polyfills.tspackages/common/src/Task.tspackages/common/src/Type.tspackages/common/src/local-first/Protocol.tspackages/common/test/Polyfills.test.tspackages/common/test/TreeShaking.test.tspackages/common/test/WebSocket.test.tspackages/common/test/_browserSetup.tspackages/common/test/_globalSetup.tspackages/common/test/local-first/Sync.test.tspackages/nodejs/package.jsonpackages/nodejs/src/Sqlite.tspackages/nodejs/src/local-first/Relay.tspackages/nodejs/test/Relay.test.tspackages/nodejs/test/Sqlite.test.tspackages/nodejs/test/setup.tspackages/react-native/package.jsonpackages/react-web/package.jsonpackages/react/package.jsonpackages/svelte/package.jsonpackages/tanstack-start/package.jsonpackages/tauri/package.jsonpackages/vue/package.jsonpackages/web/package.jsonpackages/web/test/_browserSetup.tspackages/web/vitest.config.tsscripts/rm.mtsvitest.coverage.config.mts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/common/src/Polyfills.ts (1)
119-132: Potenciální neošetřená asynchronní výjimka v detekci.Na řádku 127 je
disposeAsync()zavoláno svoid, ale výsledný Promise není ošetřen. Pokud nativní implementace vyhodí asynchronní chybu, může to způsobit varování o neošetřeném rejectu.♻️ Navrhovaná úprava pro bezpečné potlačení async chyby
const asyncDisposableStack = new AsyncDisposableStackCtor(); asyncDisposableStack.use(disposableResource); asyncDisposableStack.use(asyncDisposableResource); - void asyncDisposableStack.disposeAsync(); + void asyncDisposableStack.disposeAsync().catch(() => { + // Ignorujeme async chyby - detekujeme pouze synchronní problémy v use() + }); return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/Polyfills.ts` around lines 119 - 132, The detection block may leave an unhandled rejection from AsyncDisposableStackCtor.disposeAsync(); update the AsyncDisposableStackCtor usage (where asyncDisposableStack.use(asyncDisposableResource) and asyncDisposableStack.disposeAsync() are called) to explicitly handle/reduce the returned Promise (e.g., call disposeAsync() and attach a .catch(() => {}) or otherwise await and catch errors) so any asynchronous error is suppressed during feature detection without leaking an unhandled rejection; keep the synchronous DisposableStackCtor logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/common/src/Polyfills.ts`:
- Around line 119-132: The detection block may leave an unhandled rejection from
AsyncDisposableStackCtor.disposeAsync(); update the AsyncDisposableStackCtor
usage (where asyncDisposableStack.use(asyncDisposableResource) and
asyncDisposableStack.disposeAsync() are called) to explicitly handle/reduce the
returned Promise (e.g., call disposeAsync() and attach a .catch(() => {}) or
otherwise await and catch errors) so any asynchronous error is suppressed during
feature detection without leaking an unhandled rejection; keep the synchronous
DisposableStackCtor logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dee697cb-f331-4fa8-9d2f-b6b7674152f2
📒 Files selected for processing (1)
packages/common/src/Polyfills.ts
| "dev": "bun --watch src/index.ts", | ||
| "build": "rimraf dist && tsc", | ||
| "build": "tsc --build tsconfig.json", | ||
| "start": "node dist/src/index.js", | ||
| "clean": "rimraf .turbo node_modules dist data/evolu-relay.db" | ||
| "clean": "bun ../../scripts/rm.mts .turbo node_modules dist data/evolu-relay.db" |
There was a problem hiding this comment.
build changed from rimraf dist && tsc to tsc --build tsconfig.json without cleaning dist first. tsc --build won’t delete outputs for removed/renamed sources, so repeat builds can leave stale files under dist/. Consider adding a dist clean step (or a dedicated clean:dist) before building to keep the runtime output deterministic.
| ?.value as unknown; | ||
| const installed = | ||
| typeof installedValue === "symbol" ? installedValue : undefined; | ||
| if (installed !== undefined) return installed; |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/common/test/WebSocket.test.ts (1)
44-46:⚠️ Potential issue | 🟠 MajorBrowser WebSocket testy jsou stále fakticky vypnuté.
Na Line 44-46 je běh v browseru podmíněný
process.env.EVOLU_BROWSER_WS_TESTS === "1", ale browser konfigurace tuhle proměnnou nenastavuje, takžewsTestspadne natest.skipa browser větev se neověří.💡 Minimální oprava
-const browserWsEnabled = - typeof process !== "undefined" && process.env?.EVOLU_BROWSER_WS_TESTS === "1"; +const browserWsEnabled = typeof WebSocket !== "undefined"; const wsTest = isServer || browserWsEnabled ? test : test.skip;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/test/WebSocket.test.ts` around lines 44 - 46, The browser WebSocket branch is never exercised because browserWsEnabled only checks process.env.EVOLU_BROWSER_WS_TESTS, which isn’t set in browser builds; update the condition that defines browserWsEnabled so it detects browser runtime correctly (e.g., check typeof window !== "undefined" and read a browser-friendly env like import.meta.env.VITE_EVOLU_BROWSER_WS_TESTS or default to enabled in browser CI) and then use that in the existing wsTest assignment (symbols: browserWsEnabled, EVOLU_BROWSER_WS_TESTS, wsTest, isServer) so tests in the browser run instead of falling back to test.skip.
🧹 Nitpick comments (2)
packages/common/test/TreeShaking.test.ts (1)
201-202: Zvažte použitítest.skipIfmísto tichéhoreturn.Tiché přeskočení testu v případě, že Bun není dostupný, může v CI skrýt problém s nastavením prostředí. Vitest nabízí explicitnější API pro podmíněné přeskakování testů.
♻️ Návrh explicitního přeskočení
- test("bundle sizes (fast lane)", async () => { - if (!isBunRuntime) return; - + test.skipIf(!isBunRuntime)("bundle sizes (fast lane)", async () => { const fixtures = getFixtures();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/test/TreeShaking.test.ts` around lines 201 - 202, Currently tests use an early "if (!isBunRuntime) return;" to silently skip when Bun isn't available; replace that pattern with Vitest's conditional skip API by using test.skipIf(!isBunRuntime, ...) (or describe.skipIf for a block) so the skip is explicit in test metadata; locate occurrences of the guard (isBunRuntime and the early return) and wrap the affected test/describe callbacks with test.skipIf or describe.skipIf, preserving the original test body and name.packages/common/src/Polyfills.ts (1)
77-84: Potenciální race condition při asynchronní detekci.Mezi dokončením synchronní kontroly a vyřešením asynchronního Promise může kód mezitím použít nativní (potenciálně nefunkční) implementaci. Pokud WebKit selže pouze při async disposal (ne při synchronní probe na řádcích 118-137), kód mezi řádky 76 a 84 může stále používat nefunkční nativní API.
Toto je pravděpodobně přijatelný kompromis, protože synchronní probe zachytí většinu problémů, ale stojí za zvážení, zda by nebylo lepší instalovat owned polyfill vždy, když je detekován WebKit runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/Polyfills.ts` around lines 77 - 84, Asynchronous probe (hasBrokenNativeDisposableStackAsync) can race with code using the native API; change the initialization so the polyfill is installed deterministically: either await hasBrokenNativeDisposableStackAsync(symbolDispose, symbolAsyncDispose) during module init (remove the top-level void and make the initializer async or expose an async init function) and call installOwnedDisposableStackPolyfills(symbolDispose, symbolAsyncDispose) when it resolves true, or detect WebKit synchronously and install installOwnedDisposableStackPolyfills immediately for that runtime; update callers to await the new async init if you choose the await approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/angular-vite-pwa/package.json`:
- Around line 19-20: Aktualizujte závislost `@angular/build` v package.json z
^21.1.5 na ^21.2.1, aby byla zarovnána s ostatními Angular core balíčky (např.
`@angular/compiler-cli` na ^21.2.1); upravte hodnotu verze pro "@angular/build" v
tom bloku a uložte změnu, aby všechny core balíčky měly stejnou minor/patch
verzi.
In `@packages/common/test/TreeShaking.test.ts`:
- Around line 187-188: Normalizační rozsah pro proměnnou raw v testu (kódový
fragment upravující raw mezi 15250–15450 na 15390) nepokrývá aktuální
snapshotovou hodnotu 15511; upravte test v TreeShaking.test.ts tak, že buď
rozšíříte rozsah pro raw tak, aby zahrnoval 15511 (např. zvětšit horní hranici)
nebo odstraníte/tlumíte normalizaci pro tu konkrétní hodnotu, aby snapshot
zůstal konzistentní; odkažte se na proměnnou raw v té části testu (a případně na
související kontrolu snapshotu na řádku se 15511) při provedení změny.
In `@packages/nodejs/test/Relay.test.ts`:
- Around line 191-194: The current loose regex in the two tests uses 40[01]
which conflates 400 and 401; update each
expect(String(error.message)).toMatch(...) call to assert the exact expected
HTTP status for that specific test (replace 40[01] with the explicit status,
e.g., /Unexpected server response: 400|Connection ended/ or /Unexpected server
response: 401|Connection ended/ as appropriate), and make the same change for
the other occurrence around lines 220-223 so each test checks the precise status
rather than a range.
---
Duplicate comments:
In `@packages/common/test/WebSocket.test.ts`:
- Around line 44-46: The browser WebSocket branch is never exercised because
browserWsEnabled only checks process.env.EVOLU_BROWSER_WS_TESTS, which isn’t set
in browser builds; update the condition that defines browserWsEnabled so it
detects browser runtime correctly (e.g., check typeof window !== "undefined" and
read a browser-friendly env like import.meta.env.VITE_EVOLU_BROWSER_WS_TESTS or
default to enabled in browser CI) and then use that in the existing wsTest
assignment (symbols: browserWsEnabled, EVOLU_BROWSER_WS_TESTS, wsTest, isServer)
so tests in the browser run instead of falling back to test.skip.
---
Nitpick comments:
In `@packages/common/src/Polyfills.ts`:
- Around line 77-84: Asynchronous probe (hasBrokenNativeDisposableStackAsync)
can race with code using the native API; change the initialization so the
polyfill is installed deterministically: either await
hasBrokenNativeDisposableStackAsync(symbolDispose, symbolAsyncDispose) during
module init (remove the top-level void and make the initializer async or expose
an async init function) and call
installOwnedDisposableStackPolyfills(symbolDispose, symbolAsyncDispose) when it
resolves true, or detect WebKit synchronously and install
installOwnedDisposableStackPolyfills immediately for that runtime; update
callers to await the new async init if you choose the await approach.
In `@packages/common/test/TreeShaking.test.ts`:
- Around line 201-202: Currently tests use an early "if (!isBunRuntime) return;"
to silently skip when Bun isn't available; replace that pattern with Vitest's
conditional skip API by using test.skipIf(!isBunRuntime, ...) (or
describe.skipIf for a block) so the skip is explicit in test metadata; locate
occurrences of the guard (isBunRuntime and the early return) and wrap the
affected test/describe callbacks with test.skipIf or describe.skipIf, preserving
the original test body and name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c446d70-d8d5-4cc4-a9e0-c7495cc54a83
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
apps/relay/package.jsonexamples/angular-vite-pwa/package.jsonexamples/react-electron/package.jsonexamples/react-expo/package.jsonexamples/tanstack-start/package.jsonpackage.jsonpackages/common/src/Polyfills.tspackages/common/src/Task.tspackages/common/test/Polyfills.test.tspackages/common/test/TreeShaking.test.tspackages/common/test/WebSocket.test.tspackages/common/test/_globalSetup.tspackages/nodejs/package.jsonpackages/nodejs/test/Relay.test.tspackages/react-native/package.jsonpackages/vue/package.jsonpackages/web/package.jsonscripts/rm.mts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/common/src/Task.ts
- package.json
- examples/react-electron/package.json
- packages/nodejs/package.json
| "@angular/build": "^21.1.5", | ||
| "@angular/compiler-cli": "^21.1.5", | ||
| "@angular/compiler-cli": "^21.2.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Angular 21 @angular/build @angular/compiler-cli version compatibility peer dependencies
💡 Result:
For Angular v21 (i.e., @angular/* major 21), the relevant compatibility/peer-dependency rules are:
-
@angular/build@21.1.4peerDependencies include:@angular/compiler-cli:^21.0.0(so any 21.x satisfies it)typescript:>=5.9 <6.0- plus Angular framework peers like
@angular/core,@angular/compiler, etc. at^21.0.0(and@angular/ssrat^21.1.4). [1]
-
Angular 21 toolchain compatibility (official matrix):
- Node.js:
^20.19.0 || ^22.12.0 || ^24.0.0 - TypeScript:
>=5.9.0 <6.0.0[2]
- Node.js:
-
@angular/compiler-cliversion alignment- Angular’s core packages and CLI are major-version aligned (Angular 21 ↔ CLI 21). [3]
@angular/compiler-clirequires a matching@angular/compilerversion (pattern shown in@angular/compiler-cli@20.1.4→ peer@angular/compiler: 20.1.4; Angular 21 follows the same “match the framework version” rule). [4]
Practical takeaway: keep @angular/build, @angular/compiler-cli, @angular/compiler, @angular/core all on major 21 (ideally the same minor/patch), and use TypeScript 5.9.x (not 5.8 or 6.0). [1][2][4]
Sources: [1] [2] [3] [4]
🌐 Web query:
Angular 21.2.1 release notes package version requirements
💡 Result:
Angular v21.x (applies to 21.2.x as well): required versions
Angular’s official compatibility table for Angular 21.0.x (i.e., the v21 line) lists these requirements: Node.js ^20.19.0 || ^22.12.0 || ^24.0.0, TypeScript >=5.9.0 <6.0.0, RxJS ^6.5.3 || ^7.4.0. (angular.dev)
Key npm peer/engine requirements (example from @angular/core)
From @angular/core’s package.json (shown here for 21.1.2, but this is the same pattern used across the v21 line):
engines.node:^20.19.0 || ^22.12.0 || >=24.0.0peerDependencies.rxjs:^6.5.3 || ^7.4.0peerDependencies.zone.js:~0.15.0 || ~0.16.0peerDependencies.@angular/compiler: must match exactly (same version as@angular/core) (app.unpkg.com)
About “Angular 21.2.1 release notes”
In the official Angular monorepo tags/releases, I can see v21.2.0 (Feb 25, 2026) and a vsix-21.2.1 tag (VS Code extension), but not a framework tag v21.2.1 on the tags page. (github.com)
If you confirm whether you mean Angular framework 21.2.1 (npm packages like @angular/core@21.2.1) or the VS Code extension 21.2.1, I can point you at the exact corresponding release notes.
Citations:
- 1: https://angular.dev/reference/versions
- 2: https://app.unpkg.com/%40angular/core%4021.1.2/files/package.json?utm_source=openai
- 3: https://github.com/angular/angular/tags
🏁 Script executed:
# Zkontrolovat soubor package.json a okolní kontext
cat -n examples/angular-vite-pwa/package.json | head -30Repository: SQLoot/evolu-plan-b
Length of output: 991
Aktualizujte @angular/build na verzi ^21.2.1.
Na řádku 19 zůstává @angular/build na ^21.1.5, zatímco @angular/compiler-cli a další Angular balíčky jsou již na ^21.2.1. Podle oficiální Angular dokumentace by měly být všechny core balíčky (@angular/build, @angular/compiler-cli, @angular/compiler, @angular/core) na stejné minor/patch verzi. Ačkoli caret notace (^) technicky zajišťuje kompatibilitu, je to proti best practices.
Návrh opravy
- "@angular/build": "^21.1.5",
+ "@angular/build": "^21.2.1",📝 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.
| "@angular/build": "^21.1.5", | |
| "@angular/compiler-cli": "^21.1.5", | |
| "@angular/compiler-cli": "^21.2.1", | |
| "@angular/build": "^21.2.1", | |
| "@angular/compiler-cli": "^21.2.1", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/angular-vite-pwa/package.json` around lines 19 - 20, Aktualizujte
závislost `@angular/build` v package.json z ^21.1.5 na ^21.2.1, aby byla zarovnána
s ostatními Angular core balíčky (např. `@angular/compiler-cli` na ^21.2.1);
upravte hodnotu verze pro "@angular/build" v tom bloku a uložte změnu, aby
všechny core balíčky měly stejnou minor/patch verzi.
| if (gzip >= 5650 && gzip <= 5750) gzip = 5692; | ||
| if (raw >= 15250 && raw <= 15450) raw = 15390; |
There was a problem hiding this comment.
Rozsah normalizace nepokrývá aktuální hodnotu snapshotu.
Normalizační rozsah pro task-example raw je 15250–15450 → 15390, ale snapshot na řádku 219 obsahuje hodnotu 15511, která je mimo tento rozsah. Pokud je 15511 stabilní výstup Bun bundleru, měl by se buď rozšířit rozsah, nebo odstranit normalizace pro tuto hodnotu.
🔧 Návrh úpravy rozsahu
if (fixture === "task-example") {
if (gzip >= 5650 && gzip <= 5750) gzip = 5692;
- if (raw >= 15250 && raw <= 15450) raw = 15390;
+ if (raw >= 15400 && raw <= 15600) raw = 15511;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/test/TreeShaking.test.ts` around lines 187 - 188,
Normalizační rozsah pro proměnnou raw v testu (kódový fragment upravující raw
mezi 15250–15450 na 15390) nepokrývá aktuální snapshotovou hodnotu 15511;
upravte test v TreeShaking.test.ts tak, že buď rozšíříte rozsah pro raw tak, aby
zahrnoval 15511 (např. zvětšit horní hranici) nebo odstraníte/tlumíte
normalizaci pro tu konkrétní hodnotu, aby snapshot zůstal konzistentní; odkažte
se na proměnnou raw v té části testu (a případně na související kontrolu
snapshotu na řádku se 15511) při provedení změny.
| // Runtime-dependent: ws may report an HTTP status error or a generic close. | ||
| expect(String(error.message)).toMatch( | ||
| /Unexpected server response: 40[01]|Connection ended/, | ||
| ); |
There was a problem hiding this comment.
Zpřesněte očekávaný HTTP status v každém testu.
Aktuální regex 40[01] v obou testech dovolí i opačný status, takže regresi (400 ↔ 401) nemusí odhalit.
🎯 Návrh úpravy
- expect(String(error.message)).toMatch(
- /Unexpected server response: 40[01]|Connection ended/,
- );
+ expect(String(error.message)).toMatch(
+ /Unexpected server response: 400|Connection ended/,
+ );- expect(String(error.message)).toMatch(
- /Unexpected server response: 40[01]|Connection ended/,
- );
+ expect(String(error.message)).toMatch(
+ /Unexpected server response: 401|Connection ended/,
+ );Also applies to: 220-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nodejs/test/Relay.test.ts` around lines 191 - 194, The current loose
regex in the two tests uses 40[01] which conflates 400 and 401; update each
expect(String(error.message)).toMatch(...) call to assert the exact expected
HTTP status for that specific test (replace 40[01] with the explicit status,
e.g., /Unexpected server response: 400|Connection ended/ or /Unexpected server
response: 401|Connection ended/ as appropriate), and make the same change for
the other occurrence around lines 220-223 so each test checks the precise status
rather than a range.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Miccy <code@miccy.dev>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common/test/_globalSetup.ts`:
- Around line 15-39: The closeWithTimeout function has duplicated and incomplete
declarations causing syntax errors; fix it by keeping a single declaration of
timeout (const timeout: ReturnType<typeof setTimeout> | undefined at top of the
promise scope), define the settle function exactly once to set settled = true,
clearTimeout(timeout) and call resolve(), set timeout = setTimeout(settle,
timeoutMs) and call timeout.unref?.(), and invoke close(settle) only once;
update the function body in closeWithTimeout to follow that single, corrected
sequence so the code compiles and behaves deterministically.
- Around line 111-119: The teardown currently calls client.terminate() on each
websocket client which triggers abnormal close (1006) and causes retry logic to
run; change the loop over instance.wsServer.clients to attempt a graceful close
first by calling client.close(1001) (use an existing helper like
closeWithTimeout or implement a short timeout) and only call client.terminate()
as a fallback if the graceful close does not complete within that timeout;
ensure you still await the final Promise.all that closes wsServer and
httpServer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6f0bb87-6faf-426d-9893-39b2936d9351
📒 Files selected for processing (1)
packages/common/test/_globalSetup.ts
| const closeWithTimeout = ( | ||
| close: (callback: () => void) => void, | ||
| timeoutMs = 500, | ||
| ): Promise<void> => | ||
| new Promise<void>((resolve) => { | ||
| let settled = false; | ||
| let timeout: ReturnType<typeof setTimeout> | undefined; | ||
| const settle = () => { | ||
| if (settled) return; | ||
| let timeout: ReturnType<typeof setTimeout> | undefined; | ||
|
|
||
| const settle = () => { | ||
| if (settled) return; | ||
| settled = true; | ||
| if (timeout !== undefined) { | ||
| clearTimeout(timeout); | ||
| } | ||
| resolve(); | ||
| }; | ||
|
|
||
| timeout = setTimeout(settle, timeoutMs); | ||
| timeout.unref?.(); | ||
| close(settle); | ||
| close(settle); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/common/test/_globalSetup.ts | head -50Repository: SQLoot/evolu-plan-b
Length of output: 1782
🏁 Script executed:
wc -l packages/common/test/_globalSetup.tsRepository: SQLoot/evolu-plan-b
Length of output: 103
🏁 Script executed:
cat -n packages/common/test/_globalSetup.ts | sed -n '90,120p'Repository: SQLoot/evolu-plan-b
Length of output: 1176
🏁 Script executed:
cat -n packages/common/test/_globalSetup.ts | sed -n '15,45p'Repository: SQLoot/evolu-plan-b
Length of output: 1082
Funkce closeWithTimeout má kritické syntaktické chyby a duplicitní volání.
Blok obsahuje neúplnou definici settle na řádku 22–23 (chybí tělo funkce), poté duplicitní deklaraci timeout na řádku 24 a duplicitní kompletní definici settle na řádcích 26–33. Navíc je funkce close(settle) volána dvakrát na řádcích 37–38 místo jedenkrát, což způsobí nedeterministické chování. Kód se nepřeloží.
Navrhovaná oprava
const closeWithTimeout = (
close: (callback: () => void) => void,
timeoutMs = 500,
): Promise<void> =>
new Promise<void>((resolve) => {
let settled = false;
let timeout: ReturnType<typeof setTimeout> | undefined;
+
const settle = () => {
if (settled) return;
- let timeout: ReturnType<typeof setTimeout> | undefined;
-
- const settle = () => {
- if (settled) return;
settled = true;
if (timeout !== undefined) {
clearTimeout(timeout);
}
resolve();
};
timeout = setTimeout(settle, timeoutMs);
timeout.unref?.();
close(settle);
- close(settle);
});🧰 Tools
🪛 GitHub Actions: CI
[error] 39-39: Formatter parse error: Expected a semicolon or an implicit semicolon after a statement, but found none.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/test/_globalSetup.ts` around lines 15 - 39, The
closeWithTimeout function has duplicated and incomplete declarations causing
syntax errors; fix it by keeping a single declaration of timeout (const timeout:
ReturnType<typeof setTimeout> | undefined at top of the promise scope), define
the settle function exactly once to set settled = true, clearTimeout(timeout)
and call resolve(), set timeout = setTimeout(settle, timeoutMs) and call
timeout.unref?.(), and invoke close(settle) only once; update the function body
in closeWithTimeout to follow that single, corrected sequence so the code
compiles and behaves deterministically.
| // Ensure active sockets don't keep `wsServer.close` pending indefinitely. | ||
| for (const client of instance.wsServer.clients) { | ||
| client.terminate(); | ||
| } | ||
|
|
||
| await Promise.all([ | ||
| closeWithTimeout((done) => instance.wsServer.close(done)), | ||
| closeWithTimeout((done) => instance.httpServer.close(done)), | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Teardown implementace v _globalSetup.ts ==="
cat -n packages/common/test/_globalSetup.ts | sed -n '105,125p'
echo
echo "=== Kontextu kolem closeWithTimeout funkce ==="
rg -n -B10 -A5 'closeWithTimeout' packages/common/test/_globalSetup.ts | head -60
echo
echo "=== WebSocket.ts - close event a retry logika ==="
rg -n -B5 -A10 'onclose|shouldRetryOnClose' packages/common/src/WebSocket.ts | head -80Repository: SQLoot/evolu-plan-b
Length of output: 4102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Úplný onclose handler v WebSocket.ts ==="
rg -n -A20 'socket\.onclose = ' packages/common/src/WebSocket.ts | head -80
echo
echo "=== lazyTrue definice ==="
rg -n -B2 -A5 'lazyTrue' packages/common/src/WebSocket.ts
echo
echo "=== Jak se shouldRetryOnClose používá v retry logice ==="
rg -n -B5 -A10 'shouldRetryOnClose' packages/common/src/WebSocket.ts | grep -A10 'onclose.*shouldRetryOnClose'Repository: SQLoot/evolu-plan-b
Length of output: 2115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== lazyTrue implementace ==="
rg -n -B2 -A5 'export.*lazyTrue|const lazyTrue' packages/common/src/Function.ts
echo
echo "=== ws knihovna dokumentace v node_modules ==="
find . -name "package.json" -path "*/node_modules/ws/*" | head -1 | xargs -I {} dirname {} | head -1
echo
echo "=== Výchozí closeWithTimeout timeout ==="
grep -n "closeWithTimeout.*timeoutMs\|= 500" packages/common/test/_globalSetup.tsRepository: SQLoot/evolu-plan-b
Length of output: 496
🌐 Web query:
ws npm package terminate() vs close() behavior WebSocket closing handshake abnormal closure
💡 Result:
In the Node.js ws package:
-
ws.close([code, reason])starts the WebSocket closing handshake (it sends a Close control frame, optionally with a close code/reason). If both sides exchange Close frames, the connection is considered closed cleanly. Inws, if the peer doesn’t complete the handshake in time,wswill eventually force-close the connection aftercloseTimeout(default 30s). [1][2] -
ws.terminate()forcibly closes immediately by destroying the underlying TCP socket (socket.destroy()), without waiting for (or necessarily sending) the WebSocket Close frame/handshake. [1]
Why this often shows up as “abnormal closure” / code 1006:
- Per RFC 6455, if the TCP connection closes and no Close control frame was received, the application reports the close code as
1006(“abnormal closure”). This happens with abrupt disconnects liketerminate()(or process crash, network drop, etc.). [2] 1006is a reserved, internal-only code (it’s a reported outcome, not a code you properly send in a Close frame). [2][3]
Rule of thumb: use close() for graceful shutdown (lets the peer get a real close code/reason); use terminate() when you need an immediate teardown (e.g., stuck connection, failed heartbeat, resource protection), accepting that the peer may observe an abnormal close (1006). [1][2]
Sources:
[1] ws documentation (close(), terminate(), closeTimeout) https://github.com/websockets/ws/blob/master/doc/ws.md
[2] RFC 6455 (closing handshake; when to report 1006) https://www.rfc-editor.org/rfc/rfc6455
[3] 1006 abnormal closure overview (reserved / no close handshake) https://protocolcodes.com/websocket/1006-abnormal-closure/
Graceful close před terminate() v teardownu, aby se předešlo retry logice
client.terminate() vyvolá abnormal close event (kód 1006) bez Close handshake. Výchozí shouldRetryOnClose = lazyTrue vrací true pro všechny close eventy, což triggeruje retry logiku a způsobuje flakey chování v testech. Doporučuji nejdřív zavolat graceful client.close(1001) a terminate() pouze jako fallback, pokud se zavření nestihne za krátký timeout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/common/test/_globalSetup.ts` around lines 111 - 119, The teardown
currently calls client.terminate() on each websocket client which triggers
abnormal close (1006) and causes retry logic to run; change the loop over
instance.wsServer.clients to attempt a graceful close first by calling
client.close(1001) (use an existing helper like closeWithTimeout or implement a
short timeout) and only call client.terminate() as a fallback if the graceful
close does not complete within that timeout; ensure you still await the final
Promise.all that closes wsServer and httpServer.
This pull request introduces several improvements and refactors across the codebase, focusing primarily on replacing the use of
rimrafwith a custom Bun-based removal script, updating dependencies, and making platform compatibility enhancements. The most significant changes are grouped below.Build and Clean Script Improvements:
Replaced all uses of
rimrafincleanandbuildscripts across the monorepo with a custom Bun-based removal script (bun ../../scripts/rm.mts), improving consistency and speed for cleaning tasks. This affects rootpackage.jsonand multiple example and package scripts. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Added a
clean:tsscript at the root for cleaning TypeScript build artifacts, and updated thebuildscript to run this before building all packages. [1] [2]Platform and Compatibility Enhancements:
Improved Node.js Buffer detection and usage by introducing a
nodeBufferabstraction and updating base64 encoding/decoding logic to use this, ensuring compatibility in environments whereBuffermay not be globally available. [1] [2] [3] [4] [5]Enhanced stack trace handling in custom error classes (
BufferError,ProtocolDecodeError) to avoid errors in environments whereError.captureStackTraceis not available. [1] [2]Improved scheduler and async task compatibility by using a global
setImmediatefallback for environments where it is not defined. [1] [2]Dependency and Configuration Updates:
Updated several dependencies to newer versions, including
react-native-quick-crypto,@tauri-apps/cli, andsvelte. [1] [2] [3]Updated TypeScript configuration in
apps/relay/tsconfig.jsonto explicitly include Node types and type roots for better type checking.Removed unused dependencies such as
disposablestack,webpack, and@types/webpackfrom package manifests. [1] [2] [3]Other Improvements:
AsyncDisposableStackto better handle both synchronous and asynchronous disposables, improving resource management.These changes collectively modernize the build process, improve cross-platform compatibility, and update dependencies for better reliability and maintainability.
Summary by CodeRabbit