Expose Metrics counters as accessors - #1659
Open
TastyPi wants to merge 1 commit into
Open
Conversation
Metrics increments a counter for each Assign, Create, Clone, Discard and Update call. The counters were data properties, so the increment threw a TypeError once Metrics was frozen, which failed every schema construction in hosts that deep-freeze a shared module graph. Keeping the counters in module scope and exposing them as accessors leaves the increments working under a freeze. TMetrics is unchanged, and the counters remain readable, enumerable and spreadable. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Closes #1658
Metricsincrements a counter for everyAssign,Create,Clone,DiscardandUpdatecall. The counters are data properties, so the increment throwsTypeErroronceMetricsis frozen — and since everyType.*constructor routes through at least one of those five, schema construction fails outright rather than merely losing its metrics. The issue has a self-contained reproduction.This keeps the counters as module-scope variables and exposes them through accessors. A freeze cannot disable an accessor, so the increments keep working.
TMetricsis unchanged, andObject.keys(Metrics)and{ ...Metrics }produce identical output before and after, so the counters stay readable, enumerable and spreadable.Tests
Two tests in
test/typebox/runtime/system/memory.ts: that the counters increment, and that they still increment afterObject.freeze(Memory.Metrics).They assert the behaviour rather than the mechanism, so they do not pin the implementation to accessors specifically.
Verification
deno task lint— clean, 685 filesdeno task test— 35,188 passing, 0 failingdeno task build— cleanMetrics frozen? trueand then does not throw, wheretypebox@1.3.8throws atcreate.mjs:25Note
Happy to drop this in favour of a
Settingsflag (Settings.Set({ metrics: false })) if you would rather metrics were opt-out than freeze-tolerant — the tradeoff is that a flag has to be set before the first schema is constructed, which a library depending on TypeBox cannot guarantee.🤖 Drafted with Claude Code