Skip to content

fix: preserve frozen operation options - #425

Open
BlueX888 wants to merge 1 commit into
isaacs:mainfrom
BlueX888:contrib/preserve-frozen-operation-options
Open

BlueX888 wants to merge 1 commit into
isaacs:mainfrom
BlueX888:contrib/preserve-frozen-operation-options

Conversation

@BlueX888

Copy link
Copy Markdown

Passing frozen options to get() or another cache operation throws when the diagnostics wrapper assigns options.status, even without subscribers. Reusing mutable options also retains an automatically generated status object between calls.

Pass diagnostic status separately to the existing private methods so the caller's options remain untouched. Explicit status objects still receive updates, and inherited or non-enumerable options retain their behavior. Regression coverage exercises all seven affected operations with and without subscribers and explicit status, plus reused options.

Validation: TypeScript build, oxlint, and 409 focused assertions pass. The regression fails on unchanged main. With Node 24.19 on Windows, the full suite passes 29 test files; tracing.ts has failures on both this branch and unchanged main, so this is not a full-suite pass.

@Ekko82 Ekko82 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reused-options regression could also cover two pending fetch() calls for different keys with a subscriber on lru-cache:metrics. Using one mutable options object, base 7e71a1f gives both fetchMethod callbacks the same generated options.status; starting the second call changes the first status's key to the second key. At f75bb70, the statuses are distinct and both keys remain correct. Both fetched values resolve correctly on both revisions.

Checked on Node 24.16.0 / Windows x64 with the real diagnostics channel and Promise-backed loaders. I ran 13 public-API probes against the original TypeScript via Node's built-in transformation (base 3 pass / 10 fail; head 13 pass). I did not run the project build or TAP suite.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants