fix(zod): adopt upstream lazy-bind memory optimization (colinhacks/zod#5897)#266343
Conversation
|
Pinging @elastic/kibana-core (Team:Core) |
204e413 to
32c559b
Compare
Replaces `Object.assign(proto, methods)` in the `_initProto` helper
(added by the previous shadow-proto patch) with a `Object.defineProperty`
getter loop:
for (const [k, fn] of Object.entries(methods)) {
Object.defineProperty(proto, k, {
get() { return fn.bind(this); },
configurable: true,
enumerable: false,
});
}
When a method is accessed on an instance (e.g. `schema.optional`), the
getter returns a copy bound to `this`, so detached calls — including
destructuring and variable extraction — continue to work:
const opt = schema.optional;
opt(); // previously threw; now works correctly
The memory benefits of the shadow-proto approach are fully preserved:
builder methods are still not own properties, so V8 keeps schema
instances in fast-property mode.
Adds a behavioural unit test in `@kbn/zod` that covers the main
detached-call scenarios and documents the known trade-off (each getter
access allocates a new bound function).
Made-with: Cursor
32c559b to
378e5a2
Compare
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Async chunks
Page load bundle
History
|
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/25113849876 |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/25114569158 |
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/25114569023 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
…d#5897) (elastic#266343) ## Summary Adopts the memory optimization implemented by Colin McDonnell in [colinhacks/zod#5897](colinhacks/zod#5897) as a `patch-package` patch, ahead of the next official zod release. Also fixes [colinhacks/zod#5760](colinhacks/zod#5760). ### Background The previous patch (elastic#263121) introduced a **shadow-proto** architecture that shared builder methods on a hidden prototype layer. While it reduced per-schema heap cost by ~80%, it introduced a breaking change: **detached method calls** stopped working. ```ts const opt = schema.optional; // extracts method reference opt(); // ❌ previously threw — `this` was undefined ``` The root cause was that `_initProto` used `Object.assign` to copy methods as plain value properties onto the prototype. Detaching such a property loses the `this` context. ### This PR Replaces `_initProto` with Colin's `_installLazyMethods` approach from [colinhacks/zod#5897](colinhacks/zod#5897): ```js function _installLazyMethods(inst, group, methods) { // ... one-time setup per (proto, group) ... for (const key in methods) { const fn = methods[key]; Object.defineProperty(proto, key, { configurable: true, enumerable: false, get() { const bound = fn.bind(this); // Cache on the instance — subsequent accesses skip the getter Object.defineProperty(this, key, { configurable: true, writable: true, enumerable: true, value: bound, }); return bound; }, set(v) { Object.defineProperty(this, key, { configurable: true, writable: true, enumerable: true, value: v, }); }, }); } } ``` **How it works:** - Builder methods live as non-enumerable getters on each concrete schema's prototype - On first access per instance, the getter allocates `fn.bind(this)` and caches it as an own property — subsequent accesses skip the getter entirely - Detached calls (`const m = schema.optional; m()`) work because `m` is a bound function with `this` already resolved - Parse-family methods (`parse`, `safeParse`, etc.) stay as eager per-instance closures — they're the hot path and most-detached methods **Memory characteristics are unchanged:** builder methods are not own properties until first accessed, so V8 keeps schema instances in fast-property mode. Per Colin's numbers: -40% heap per instance, -25% construction time vs Zod 4.3.6 baseline. ## Why `patch-package` Upstream zod CI has been broken for months and there is no scheduled release. `patch-package` allows us to ship the fix now. Once a new zod version including [colinhacks/zod#5897](colinhacks/zod#5897) is released, this patch can be dropped. ## Changes - `patches/zod+4.3.6.patch` — replaces the previous shadow-proto patch with Colin's `_installLazyMethods` approach (only touches `v4/classic/schemas.cjs` and `v4/classic/schemas.js`) - `src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts` — behavioural unit tests proving detached calls work and that the lazy-cache correctly returns the same bound function on repeated access ## Test plan - [x] `node scripts/jest src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts` — 5/5 pass - [ ] Full CI green --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit a9ecabf)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…cks/zod#5897) (#266343) (#266401) # Backport This will backport the following commits from `main` to `9.4`: - [fix(zod): adopt upstream lazy-bind memory optimization (colinhacks/zod#5897) (#266343)](#266343) <!--- Backport version: manual --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) Made with [Cursor](https://cursor.com) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
|
Zod 4.4 has been released |
…types (#267326) ### Summary Cleanup pass on `@kbn/evals-common` and the `evals` plugin, mirroring what was done for `@kbn/inbox-common` in #265634. No functional changes for end users, but the OAS docs generated from these routes are now correct, and the in-memory cost of the Zod schemas drops via the upstream lazy-bind optimization. ### Changes - **Switch to canonical `buildRouteValidationWithZod` from `@kbn/zod-helpers/v4`** - Deleted the local copy in `kbn-evals-common/impl/schemas/common.ts` and its re-export from `kbn-evals-common/index.ts`. - Updated all 18 evals route files to import the helper from `@kbn/zod-helpers/v4`. - Pruned `@kbn/zod-helpers` and `@kbn/core` from `kbn-evals-common`'s `moon.yml` / `tsconfig.json`; added `@kbn/zod-helpers` to the `evals` plugin's instead. - The canonical helper attaches `_sourceSchema` to the returned validator (added in #263354), which `kbn-router-to-openapispec` unwraps so route `params` / `query` / `body` actually appear in generated OAS docs. The local copy did not, so any OAS doc generation for these routes was silently dropping schema info. - **Regenerated OAS types** (`yarn openapi:generate` in `kbn-evals-common`) - Picks up the `lazySchema(() => …)` wrappers introduced by #264125 and tuned in #266343 (upstream `colinhacks/zod#5897` lazy-bind memory optimization). 19 `.gen.ts` files updated; pattern matches the prior inbox regen exactly. - **README cleanup** — pointed at the new helper location and dropped the obsolete "after regenerating, you may need to fix unused imports" workaround (no longer needed thanks to the OAS-generator `fix_eslint.ts` fix from #265634, which forces the non-editor branch when invoked from agent/IDE terminals). _PR developed with Cursor + Claude Opus 4.7 Super Duper xHigh Thinking++_
Summary
Adopts the memory optimization implemented by Colin McDonnell in colinhacks/zod#5897 as a
patch-packagepatch, ahead of the next official zod release. Also fixes colinhacks/zod#5760.Background
The previous patch (#263121) introduced a shadow-proto architecture that shared builder methods on a hidden prototype layer. While it reduced per-schema heap cost by ~80%, it introduced a breaking change: detached method calls stopped working.
The root cause was that
_initProtousedObject.assignto copy methods as plain value properties onto the prototype. Detaching such a property loses thethiscontext.This PR
Replaces
_initProtowith Colin's_installLazyMethodsapproach from colinhacks/zod#5897:How it works:
fn.bind(this)and caches it as an own property — subsequent accesses skip the getter entirelyconst m = schema.optional; m()) work becausemis a bound function withthisalready resolvedparse,safeParse, etc.) stay as eager per-instance closures — they're the hot path and most-detached methodsMemory characteristics are unchanged: builder methods are not own properties until first accessed, so V8 keeps schema instances in fast-property mode. Per Colin's numbers: -40% heap per instance, -25% construction time vs Zod 4.3.6 baseline.
Why
patch-packageUpstream zod CI has been broken for months and there is no scheduled release.
patch-packageallows us to ship the fix now. Once a new zod version including colinhacks/zod#5897 is released, this patch can be dropped.Changes
patches/zod+4.3.6.patch— replaces the previous shadow-proto patch with Colin's_installLazyMethodsapproach (only touchesv4/classic/schemas.cjsandv4/classic/schemas.js)src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts— behavioural unit tests proving detached calls work and that the lazy-cache correctly returns the same bound function on repeated accessTest plan
node scripts/jest src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts— 5/5 pass