perf(import): tree-shake remeda (v2 + named imports)#180
Merged
Conversation
…king `import * as remeda from "remeda"` cannot be tree-shaken, so all 164 of remeda's per-function modules landed in every static import closure (~53% of the slim ./client entry, per #178). remeda v1 does not tree-shake even with named imports (its `export *` barrel plus the TS namespace-merge `.strict` IIFE pattern defeat rolldown), so this bumps remeda to v2 (whose ESM story supports it) and converts every `import * as remeda` site in src/ and test/ to specific named imports. Result: the index static closure drops from 164 remeda modules to 32 (only the ~30 functions actually used, plus their shared helpers). v2 API migration: - `.strict` variants removed (strict is the default): `keys.strict` -> `keys`, `fromEntries.strict` -> `fromEntries`, `sortBy.strict` -> `sortBy`, `groupBy.strict` -> `groupBy` - `isDefined.strict` (narrows null+undefined) -> `isNonNullish` - `maxBy(a, fn)` -> `firstBy(a, [fn, "desc"])`, `minBy(a, fn)` -> `firstBy(a, fn)` - `flatten`/`flattenDeep` -> `flat` - `unique<T>`/`difference<T>` type-param is the container in v2; dropped it - `keysToOmitFromSignedPropertyNames` is now an `as const` readonly tuple so v2's `omit`/`pick` (keys typed `const Keys extends readonly ...`) remove keys precisely; a plain array leaked keys into the derived zod .pick() types - explicit annotations/casts where v2's PickFromArray/EnumeratedPartial branded return types replaced clean Pick/Omit (remote-community, rpc-local-community, signatures, util) Local-name collisions handled by aliasing only the 3 genuine ones (pages/util `keys`, db-handler `entries`, validatecomment `clone`).
remeda v2's `difference` is multiset (removes each `other` element once), whereas v1's was set-based (removes all matching occurrences). The `*ReservedFields` lists concatenate keys from multiple schemas plus explicit literals, so a field that appears twice in the candidate list AND is a signed field (present in the pubsub schema) survived as reserved under v2 — CommentModerationReservedFields regained commentModeration/communityPublicKey/ communityName and VotePubsubReservedFields regained `vote`. That made every moderation and vote publication fail challenge verification with "has a reserved field". Wrap the affected candidate lists in `unique()` (matching the guard comment-edit's list already had) so the multiset difference is equivalent to v1's set difference. Also guard the two runtime set-difference sites (ipns-publishing cids-to-unpin, util deleted-communities) the same way — the unpin one could otherwise unpin a page cid still referenced by another sort. `intersection` is also multiset in v2 but every call site is a `.length > 0` membership check, so it is unaffected.
📝 WalkthroughWalkthroughThis PR upgrades ChangesRemeda v2 Migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…tance remeda v2's clone() delegates to structuredClone() for class instances, which throws DataCloneError on a live Comment (EventEmitter + function refs). Spread the source into a plain object instead; createComment() builds a fresh instance from it, so nulling raw.comment can't corrupt the shared source. Fixes the 4 node-local failures in this suite.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stats.ts (1)
62-86: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
scoremust be synchronous here —sortBycompares the returned value directly, so it ends up sorting Promises instead of gateway scores.Precompute the scores first (for example with
Promise.all) and then sort on the resolved numeric score.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stats.ts` around lines 62 - 86, The `sortGatewaysAccordingToScore` method is passing an async `score` callback into `sortBy`, so it sorts Promises instead of numeric scores. Make `score` synchronous by preloading each gateway’s failure/success data first (for example using `Promise.all` in `sortGatewaysAccordingToScore`), then sort `gateways` using the resolved numeric score from `_gatewayScore`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/stats.ts`:
- Around line 62-86: The `sortGatewaysAccordingToScore` method is passing an
async `score` callback into `sortBy`, so it sorts Promises instead of numeric
scores. Make `score` synchronous by preloading each gateway’s failure/success
data first (for example using `Promise.all` in `sortGatewaysAccordingToScore`),
then sort `gateways` using the resolved numeric score from `_gatewayScore`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: db3f27f8-094c-4f1a-82e1-c8f3b5cbc166
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (83)
docs/protocol/import-performance.mdpackage.jsonsrc/clients/base-client-manager.tssrc/community/community-client-manager.tssrc/community/community-gateway-selection.tssrc/community/community-wire.tssrc/community/remote-community.tssrc/community/rpc-local-community.tssrc/community/rpc-remote-community.tssrc/community/schema.tssrc/pages/pages-client-manager.tssrc/pages/util.tssrc/pkc/pkc-client-manager.tssrc/pkc/pkc.tssrc/publications/comment-edit/schema.tssrc/publications/comment-moderation/schema.tssrc/publications/comment/comment-client-manager.tssrc/publications/comment/comment.tssrc/publications/comment/schema.tssrc/publications/community-edit/schema.tssrc/publications/publication-author.tssrc/publications/publication-client-manager.tssrc/publications/publication.tssrc/publications/vote/schema.tssrc/publications/vote/vote.tssrc/pubsub-messages/schema.tssrc/rpc/src/index.tssrc/runtime/browser/localforage-lru.tssrc/runtime/node/addresses-rewriter-proxy-server.tssrc/runtime/node/community/challenges/index.tssrc/runtime/node/community/challenges/pkc-js-challenges/publication-match.tssrc/runtime/node/community/db-handler.tssrc/runtime/node/community/local-community.tssrc/runtime/node/community/local-community/challenges.tssrc/runtime/node/community/local-community/comment-updates.tssrc/runtime/node/community/local-community/db-state.tssrc/runtime/node/community/local-community/editing.tssrc/runtime/node/community/local-community/ipns-publishing.tssrc/runtime/node/community/local-community/lifecycle.tssrc/runtime/node/community/local-community/publication-store.tssrc/runtime/node/community/local-community/publication-validation.tssrc/runtime/node/community/local-community/reprovide-on-address-change.tssrc/runtime/node/community/page-generator.tssrc/runtime/node/setup-kubo-address-rewriter-and-http-router.tssrc/runtime/node/sqlite-lru-cache.tssrc/runtime/node/util.tssrc/schema/schema.tssrc/signer/constants.tssrc/signer/signatures.tssrc/stats.tssrc/test/test-util.tssrc/util.tstest/challenges/challenges.test.tstest/challenges/exclude.test.tstest/challenges/publication-match.test.tstest/challenges/voucher.test.tstest/node-and-browser/community/createcommunity.pkc.test.tstest/node-and-browser/community/posts/pages.posts.test.tstest/node-and-browser/community/update.community.test.tstest/node-and-browser/community/wire-format-migration.test.tstest/node-and-browser/publications/comment-edit/content.edit.test.tstest/node-and-browser/publications/comment-edit/delete.edit.test.tstest/node-and-browser/publications/comment-moderation/pin.test.tstest/node-and-browser/publications/comment-moderation/purged.test.tstest/node-and-browser/publications/comment-moderation/remove.test.tstest/node-and-browser/publications/comment/backward.compatibility.commentupdate.test.tstest/node-and-browser/publications/comment/publish/publish.verification.test.tstest/node-and-browser/publications/community-edit/community.edit.publication.test.tstest/node-and-browser/publications/vote/downvote.test.tstest/node-and-browser/publications/vote/upvote.test.tstest/node-and-browser/pubsub-msgs/backward.compatibility.pubsub.test.tstest/node-and-browser/signatures/comment.test.tstest/node-and-browser/signatures/community.test.tstest/node-and-browser/signatures/edit.comment.test.tstest/node-and-browser/signatures/pages.test.tstest/node-and-browser/signatures/pubsub.messages.test.tstest/node-and-browser/signatures/vote.test.tstest/node/comment/post/pages.posts.test.tstest/node/community/create.community.test.tstest/node/community/edit.community.test.tstest/node/community/page-generation/edgecases.page.generation.community.test.tstest/node/community/purge-reserved-fields.db.community.test.tstest/node/pkc/validatecomment.pkc.test.ts
💤 Files with no reviewable changes (1)
- src/publications/vote/vote.ts
The comment.stop() abort tests asserted expect(errors).to.have.length(0), but the update loop legitimately emits a retriable ERR_FAILED_TO_FETCH_COMMENT_UPDATE_FROM_ALL_POST_UPDATES_RANGES error while polling for the freshly-published comment's CommentUpdate (not yet propagated to the remote gateway). On slow configs (firefox + remote-ipfs-gateway) that error lands in the update()->stop() window, flaking the assertion. The abort itself is correctly swallowed and never emits. Filter out the orthogonal transient error before asserting the abort surfaced nothing.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/node-and-browser/publications/comment/stop.comment.test.ts (1)
15-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a runtime
instanceofcheck instead of a type assertion.
(e as PKCError).codeis a compile-time-only assertion; it doesn't verifyeis actually aPKCErrorat runtime. It happens to be safe here since a non-PKCErrorwould just have.code === undefined(still excluded correctly), but it's fragile if error shapes change. SincePKCErroris currentlyimport type-only, usinginstanceofwould require importing the actual class instead.♻️ Optional refactor using a runtime check
-import type { PKCError } from "../../../../dist/node/pkc-error.js"; +import { PKCError } from "../../../../dist/node/pkc-error.js"; ... function errorsExcludingTransientUpdateFetchFailures(errors: Error[]) { - return errors.filter((e) => (e as PKCError).code !== "ERR_FAILED_TO_FETCH_COMMENT_UPDATE_FROM_ALL_POST_UPDATES_RANGES"); + return errors.filter( + (e) => !(e instanceof PKCError && e.code === "ERR_FAILED_TO_FETCH_COMMENT_UPDATE_FROM_ALL_POST_UPDATES_RANGES") + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node-and-browser/publications/comment/stop.comment.test.ts` around lines 15 - 26, The transient-error filter in errorsExcludingTransientUpdateFetchFailures currently relies on a type assertion instead of a runtime check. Replace the compile-time-only cast on e with a real instanceof check against PKCError, which means switching the current import to a runtime class import in stop.comment.test.ts. Keep the filter behavior the same by still excluding only errors whose code matches ERR_FAILED_TO_FETCH_COMMENT_UPDATE_FROM_ALL_POST_UPDATES_RANGES.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/node-and-browser/publications/comment/stop.comment.test.ts`:
- Around line 15-26: The transient-error filter in
errorsExcludingTransientUpdateFetchFailures currently relies on a type assertion
instead of a runtime check. Replace the compile-time-only cast on e with a real
instanceof check against PKCError, which means switching the current import to a
runtime class import in stop.comment.test.ts. Keep the filter behavior the same
by still excluding only errors whose code matches
ERR_FAILED_TO_FETCH_COMMENT_UPDATE_FROM_ALL_POST_UPDATES_RANGES.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 82868014-e922-4a88-8712-d3aaba0ec4dc
📒 Files selected for processing (1)
test/node-and-browser/publications/comment/stop.comment.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the import-performance work (#120 / #178). Tackles the remeda step written up in #178.
Problem
Every module did
import * as remeda from "remeda", and a namespace import cannot be tree-shaken, so all 164 of remeda's per-function modules landed in every static import closure — ~53% of the slim./cliententry's 309 modules (#178).remeda v1 does not tree-shake even with named imports: its
export *barrel plus the TypeScript namespace-merge.strictIIFE pattern defeat rolldown. Verified with a throwaway rolldown build — named imports of a handful of functions still pulled all 164 modules. (A re-export barrel was tried in #178 and also gave zero reduction.)remeda v2 tree-shakes cleanly (10 modules for a 5-function import in the throwaway build), so this bumps remeda to
^2.39.0and converts everyimport * as remedasite insrc/(49 files) andtest/(31 files) to specific named imports (remeda.pick→pick).Result
./clientstatic closure: 309 → 167 modules (remeda 164 → 22).index static closure: remeda 164 → 32 modules./clientimport ~1274ms → ~1218ms (~4-5%); the full.index entry moved within noise (~2049ms → ~2021ms). The win is the much smaller module graph, a prerequisite for pushing./clienttoward sub-second.v2 API migration
.strictvariants removed (strict is the default):keys.strict/fromEntries.strict/sortBy.strict/groupBy.strictdrop.strict;isDefined.strict(null+undefined) →isNonNullishmaxBy(a, fn)→firstBy(a, [fn, "desc"]),minBy(a, fn)→firstBy(a, fn)flatten/flattenDeep→flatunique<T>/difference<T>type param is the container in v2 — droppedkeysToOmitFromSignedPropertyNamesis now anas consttuple: v2'somit/picktype key params asconst Keys extends readonly ..., so a plain array leaked keys into the derived zod.pick()schema typesPickFromArray/EnumeratedPartialbranded return types replaced cleanPick/Omit(remote-community, rpc-local-community, signatures, util)Runtime behavior change caught in testing (2nd commit)
v2's
differenceis multiset (removes eachotherelement once) where v1's was set-based (removes all matches). The*ReservedFieldslists concatenate keys from several schemas plus literals, so a field appearing twice in the candidate list and signed (present in the pubsub schema) survived as reserved —CommentModerationReservedFieldsregained commentModeration/communityPublicKey/communityName andVotePubsubReservedFieldsregainedvote, which made every moderation and vote publication fail challenge verification with "has a reserved field". Fixed by wrapping the affected candidate lists inunique()(matching the guard comment-edit's list already had), plus the two runtime set-difference sites (cids-to-unpin, deleted-communities).intersectionis also multiset in v2 but every call site is a.length > 0membership check, so it is unaffected.Local-name collisions were handled by aliasing only the 3 genuine ones (pages/util
keys, db-handlerentries, validatecommentclone).Validation
npm run build(tsc + rolldown bundle) andconfig/verify-bundle.jspass;npx tsc --project test/tsconfig.json --noEmitpassesscripts/smoke-pack-install.jspasses (pack + install + createCommunity lifecycle, incl. inlined-deps-hidden pass)pkc(local-kubo-rpc + remote-pkc-rpc),rpc.errors(USE_RPC),pkc-ws-serverall 22 methods (comment/vote/moderation/edit publishing),ban-then-purgemoderation, vote upvote + backward-compat, comment-edit backward-compat — all green*ReservedFields/*SignedPropertyNamesexport between the v1 and v2 builds; membership is identical after theunique()fixSummary by CodeRabbit
New Features
Bug Fixes
Documentation