Repository navigation
Speed up assignProperties with an Object.assign fast path - #9511
Merged
Merged
Conversation
assignProperties (used by Data.Class/TaggedClass, Data.Error/TaggedError, Request, Persistable and OpenApi field copying) looped over Reflect.ownKeys and assigned one property at a time so that an own "__proto__" key landed as a safe data property instead of triggering the inherited __proto__ setter. For the common case - no own "__proto__" key - that loop can be replaced with a single native Object.assign call, which copies the same own+enumerable keys in the same order with the same [[Get]]/[[Set]] semantics. Micro-benchmark (Node 22.16, 5x2,000,000 iterations, 5-field class instance): old loop ~206 ns/op median vs fast path ~42 ns/op median, ~4.9x. A V8 %HaveSameMap check confirms both paths converge on the same hidden class for the same field shape. Adds a differential test suite in test/Data.test.ts comparing the fast path against a reimplementation of the old loop across getters, symbols, non-enumerable keys, frozen/setter/read-only targets, null-prototype and JSON.parse'd own-"__proto__" sources, plus a documented, deliberately narrow known limitation: a Proxy source whose getOwnPropertyDescriptor trap answers inconsistently across calls can still defeat the guard, since it queries the source twice (once here, once inside Object.assign). This does not affect any real call site in this codebase, all of which pass plain objects or JSON.parse output. Co-Authored-By: Claude Sonnet 5 <[email protected]>
🦋 Changeset detectedLatest commit: cef5fc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![effect-janitor[bot]](https://avatars.githubusercontent.com/in/4811166?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review complete: one blocking prototype-safety regression confirmed. Validated the reproduction against the changed helper; the Vitest suite was not run because dependencies are not installed.
Reviewed at cf80af8: 1 blocking, 0 nits.
Disagree? Reply in the finding's thread, or a maintainer can comment /janitor to re-run or dismiss this review.
Replace the differential matrix and the internal-module tests with two Data.Class tests: own enumerable string/symbol keys are copied and non-enumerable keys are skipped, and an own __proto__ key from JSON.parse lands as a data property without replacing the instance prototype.
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
This was referenced Oct 25, 2026
brandhaug
added a commit
to brandhaug/b2b-saas-starter
that referenced
this pull request
Oct 25, 2026
## pnpm-workspace.yaml (default) ## Dependency Updates | Package | From | To | Type | | --- | --- | --- | --- | | `@effect/platform-node` | 4.0.5 | 4.0.6 | patch | | `@tanstack/react-router` | 1.170.45 | 1.170.46 | patch | | `@tanstack/react-start` | 1.169.3 | 1.169.4 | patch | | `ai` | 7.0.148 | 7.0.149 | patch | | `effect` | 4.0.5 | 4.0.6 | patch | | `effectful-better-auth` | 1.0.7 | 1.0.8 | patch | ## Release Notes <details> <summary><b>@<!---->effect/platform-node</b> (4.0.5 → 4.0.6)</summary> ### Patch Changes - [#9506](Effect-TS/effect#9506) [`679f956`](Effect-TS/effect@679f956) Thanks @<!---->tim-smart! - Use `Effect.fnUntraced` for internal functions that returned `Effect.gen`. - [#9262](Effect-TS/effect#9262) [`51930c0`](Effect-TS/effect@51930c0) Thanks @<!---->effect-bot! - Return a typed multipart parse error instead of a defect when a multipart request is missing its boundary. - [#9402](Effect-TS/effect#9402) [`d1201f0`](Effect-TS/effect@d1201f0) Thanks @<!---->effect-bot! - Release writables created by `NodeSink.fromWritable` and `fromWritableChannel` when upstream fails or the sink is interrupted, preserving the original exit cause. Writables using `endOnDone: false` remain open. Preserve stream body encoding errors in the Node HTTP client instead of reporting transport errors caused by request cleanup. - [#9575](Effect-TS/effect#9575) [`c29ddd8`](Effect-TS/effect@c29ddd8) Thanks @<!---->tim-smart! - Add streaming request payloads to `HttpApi`. An endpoint can declare `HttpApiSchema.StreamUint8Array` or the new `HttpApiSchema.StreamNdjson({ schema })` as its `payload`, and the handler receives a lazy `Stream` that reads the body as it pulls. Bad NDJSON lines fail the stream with `HttpApiSchemaError` (400). The new `maxBytes` and `maxElementBytes` limits fail it with the new `HttpApiPayloadTooLarge` (413), and a `content-length` over `maxBytes` is rejected before the handler runs. `StreamNdjson` also works as a success response. `HttpApiClient` sends a `Stream` payload without buffering it and infers the stream's requirements. `HttpApiTest` passes stream payloads through unbuffered. `OpenApi.fromApi` marks streaming request bodies with `x- …[full notes](https://github.com/Effect-TS/effect/releases/tag/%40effect/platform-node%404.0.6) </details> <details> <summary><b>@<!---->tanstack/react-router</b> (1.170.45 → 1.170.46)</summary> ### Patch Changes - [#8670](TanStack/router#8670) [`32a3360`](TanStack/router@32a3360) - Reference agent skills in other packages by their `intent load` identifier (`@tanstack/<package>#<skill>`) instead of relative file links that break once packages are installed. - [#8671](TanStack/router#8671) [`689a7c5`](TanStack/router@689a7c5) - Publish the `@tanstack/react-router` agent skills (`react-router`, `compositions/router-query`, `lifecycle/migrate-from-react-router`), which were missing from the package's `files` list. - Updated dependencies \[[`32a3360`](TanStack/router@32a3360)]: - @<!---->tanstack/[email protected] </details> <details> <summary><b>@<!---->tanstack/react-start</b> (1.169.3 → 1.169.4)</summary> ### Patch Changes - [#8670](TanStack/router#8670) [`32a3360`](TanStack/router@32a3360) - Reference agent skills in other packages by their `intent load` identifier (`@tanstack/<package>#<skill>`) instead of relative file links that break once packages are installed. - Updated dependencies \[[`32a3360`](TanStack/router@32a3360), [`689a7c5`](TanStack/router@689a7c5), [`ed55775`](TanStack/router@ed55775), [`63a886d`](TanStack/router@63a886d), [`d9b07e8`](TanStack/router@d9b07e8)]: - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] - @<!---->tanstack/[email protected] </details> <details> <summary><b>effect</b> (4.0.5 → 4.0.6)</summary> ### Patch Changes - [#9674](Effect-TS/effect#9674) [`ce13a48`](Effect-TS/effect@ce13a48) Thanks @<!---->effect-bot! - Allow `Response.AllParts`, `Response.Part` and `Response.StreamPart` to be called without a toolkit, decoding tool call and tool result parts for any tool name with `unknown` parameters and results. - [#9696](Effect-TS/effect#9696) [`f3bbbb4`](Effect-TS/effect@f3bbbb4) Thanks @<!---->gcanti! - Fix `Array.unionWith` calling custom equivalence callbacks with values from the wrong input or in the wrong order. - [#9511](Effect-TS/effect#9511) [`5547315`](Effect-TS/effect@5547315) Thanks @<!---->sledorze! - Speed up field copying in `Data.Class`, `Data.Error`, `Request.Class` and `Persistable` constructors. - [#9743](Effect-TS/effect#9743) [`6093cfb`](Effect-TS/effect@6093cfb) Thanks @<!---->effect-bot! - Fix `AsyncResult` equality treating unrelated `Equal` values with a matching tag and hash as equal. - [#9607](Effect-TS/effect#9607) [`30319d5`](Effect-TS/effect@30319d5) Thanks @<!---->effect-bot! - Fix Effect atoms dropping the latest exposed success when an asynchronous run fails, so an `initialValue` or a success published with `get.setSelf` remains available as the failure's previous success. - [#9313](Effect-TS/effect#9313) [`74a7596`](Effect-TS/effect@74a7596) Thanks @<!---->effect-bot! - Preserve Reactivity and global layers when replacing an atom runtime layer through registry initial values, so injected test layers retain shared invalidation and global services. - [#9190](https://github.com …[full notes](https://github.com/Effect-TS/effect/releases/tag/effect%404.0.6) </details> --- *This PR was auto-generated by [catalog-update-action](https://github.com/brandhaug/catalog-update-action).* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This branch is waiting to be deployed
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
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.
assignProperties(used byData.Class/TaggedClass,Data.Error/TaggedError,Request.Class,Persistableand OpenApi field copying) loops overReflect.ownKeysand assigns one property at a time, so that an own"__proto__"key lands as a data property instead of firing the inherited__proto__setter.When the source has no own
"__proto__"key (Object.hasOwn), that loop is replaced with a singleObject.assign, which for ordinary objects copies the same own enumerable string and symbol keys in the same order with the same[[Get]]/[[Set]]semantics, including throwing on frozen targets and stopping at the first throwing getter. Sources with an own"__proto__"key (e.g.JSON.parseoutput) keep using the per-property loop.Micro-benchmark, 5-field class instance, 2,000,000 iterations per round, medians: ~206 ns/op → ~42 ns/op on Node 22.16 (author); ~2030 ns/op → ~330 ns/op on a slower Node 26 box (reviewer). Both ~5-6x.
Limitation: callers forward user-supplied objects, including Proxies. The guard adds a
getOwnPropertyDescriptor("__proto__")lookup before key enumeration and copying, so Proxy traps can observe it, throw or mutate state. Proxies with stable, side-effect-free traps retain the copying behavior, but full Proxy behavioral equivalence is not guaranteed: inconsistent descriptor answers can let an own__proto__key reach the inherited setter. This is an accepted compatibility tradeoff under the untrusted-data prototype-pollution threat model (such as parsed JSON), not protection against arbitrary in-process code.Tests:
Data.Classcopies own enumerable string and symbol keys and skips non-enumerable ones; an own__proto__key fromJSON.parselands as a data property without replacing the instance prototype (fails against a bareObject.assign).Closes EFF-2450