Skip to content

Add configure option to disable descriptor cache#4628

Closed
anchmelev wants to merge 2 commits intomobxjs:mainfrom
anchmelev:fix/descriptor-cache-4555
Closed

Add configure option to disable descriptor cache#4628
anchmelev wants to merge 2 commits intomobxjs:mainfrom
anchmelev:fix/descriptor-cache-4555

Conversation

@anchmelev
Copy link
Copy Markdown

This PR adds useDescriptorCache to configure.

By default, MobX caches property descriptors for observable object keys so repeated keys can reuse the same getter/setter pair. That behavior stays unchanged.

The issue is that in workloads with effectively unbounded dynamic keys, such as UUID-based object dictionaries on SSR servers, the module-level descriptorCache can grow without bound and retain descriptors for keys that will never be reused.

This change adds a configuration option to disable descriptor reuse for future observable object properties. When disabled, MobX also clears the existing descriptor cache so already accumulated entries can be released.

observable.map is still the preferred model for truly dynamic key-based collections. This change is meant as an escape hatch for cases where switching data structures is not immediately possible.

Code change checklist

  • Added/updated unit tests
  • Updated /docs. Added useDescriptorCache to docs/configuration.md
  • Verified that there is no significant performance drop (yarn mobx test:performance)

Refs: #4555

Add useDescriptorCache to configure so apps can opt out of descriptor reuse for observable plain-object keys.

When disabled, MobX also clears the existing descriptor cache to avoid retaining descriptors for unbounded dynamic keys.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 17, 2026

🦋 Changeset detected

Latest commit: 4b43010

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mobx Minor

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

@anchmelev
Copy link
Copy Markdown
Author

Gentle follow-up on this PR: would appreciate a review when someone has a chance.
This adds an opt-out for descriptor cache reuse to address the unbounded dynamic-key case from #4555, while keeping the default behavior unchanged.

@kubk
Copy link
Copy Markdown
Collaborator

kubk commented Mar 20, 2026

@anchmelev Thank you for the PR and digging into this.

While I understand it can be a perf improvement I dont think that's a good direction. The issue here is that the observable plain objects are the wrong data structure for unbounded dynamic keys. We explicitly warn about that in the docs: https://mobx.js.org/observable-state.html

Screenshot 2026-03-20 at 13 19 21

If we need to tell users "for this workload use some extra option to avoid trouble" then i'd rather just tell them to use observable.map instead of a plain object.

configure is already at risk of becoming a graveyard of niche escape hatches. Each new public API option adds a maintenance burden in the future

@anchmelev
Copy link
Copy Markdown
Author

@anchmelev Thank you for the PR and digging into this.

While I understand it can be a perf improvement I dont think that's a good direction. The issue here ins't that descriptorCache is bad, it's that observable plain objects are the wrong data structure for unbounded dynamic keys. We explicitly warn about that in the docs: https://mobx.js.org/observable-state.html

Screenshot 2026-03-20 at 13 19 21 If we need to tell users "for this workload use some extra option to avoid trouble" then i'd rather just tell them to use observable.map instead of a plain object.

configure is already at risk of becoming a graveyard of niche escape hatches. Each new public API option adds a maintenance burden in the future

Thanks, that makes sense.

I agree the core issue is using observable plain objects for unbounded dynamic keys, and I understand the concern about adding another niche escape hatch to configure.

I’ll drop this approach.

@anchmelev anchmelev closed this Mar 20, 2026
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