MemberJunction / MemberJunction/MJ
Fail-fast guard for the shared metadata graph: freeze Info objects (at least in dev) after Config
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Context
Since #3083 / PR #3087, providers configured with `ignoreExistingMetadata: false` (MJServer's per-request RW/RO providers) share the global provider's metadata **Info object instances** by reference instead of deep-cloning the graph. The shell gives each provider its own top-level array containers (so `provider.Entities.sort(...)` etc. stays request-local), but everything below that level is shared process-wide:
- property writes on shared Info objects (`EntityInfo`, `EntityFieldInfo`, `ApplicationInfo`, ...)
- in-place mutation of their **nested arrays** (`entity.Fields`, `entity.RelatedEntities`, `application.ApplicationEntities`, ...)
The contract is "treat Info objects and everything they own as read-only" — but today that contract is **convention-only**. A repo-wide sweep during #3087 review found zero violators (the one in-place `provider.Entities` sort was fixed in that PR), but nothing stops a future MJ change or a downstream app's custom resolver from introducing one. The failure mode is silent cross-request/process-wide state corruption — exactly the class of bug that's cheap to prevent and expensive to debug.
## Proposal
Add a fail-fast guard so an illegal write throws at the mutation site instead of corrupting shared state:
1. **`Object.freeze` (deep) the metadata graph after the global provider's `Config()`/`Refresh()` completes** — the Info objects and their nested arrays, not just the top-level containers. Refreshes swap the whole `AllMetadata` object via `UpdateLocalMetadata`, so freezing the finished graph doesn't interfere with refresh flows.
2. **Consider dev-mode-only first.** Freezing changes observable behavior for any existing (out-of-repo) code that mutates Info objects on a client's global provider — such writes currently "work" (process-visible but tolerated). Gating the freeze behind an env flag / non-production default gets the bug-catching benefit without a compat cliff; promoting to always-on can be a later decision.
## Open questions
- **Lazy caches:** several Info classes memoize derived values post-construction (deterministic, provider/user-independent). A blanket deep freeze breaks those. Options: compute them eagerly before freezing, back them with a non-frozen side map (`WeakMap`), or exclude those specific slots from the freeze.
- **Perf:** deep-freezing a 600-entity graph is a one-time cost at Config/Refresh (not per-request) — should be measured but is expected to be negligible relative to the metadata load itself.
- **Scope:** the freeze should apply to the graph itself (shared by every shell), so it protects the client global-provider case too, not just server per-request providers.
## References
- #3083 (the per-request deep-clone tax), PR #3087 (the shared-shell fix — see its description's "one behavioral contract" section, where this guard was explicitly proposed as a deliberate follow-up rather than being bundled into the perf fix)
- `CreateSharedMetadataShell` / `CopyMetadataFromGlobalProvider` in `packages/MJCore/src/generic/providerBase.ts`
- The one real-world violator found and fixed: `entity-selector-with-grid.component.ts` in-place sort of `provider.Entities`
Contributor guide
Research direction
Start in packages/MJCore/src/generic/providerBase.ts at CreateSharedMetadataShell and CopyMetadataFromGlobalProvider, then trace the global provider's Config()/Refresh() flow through UpdateLocalMetadata. Review the Info classes for lazy caches and determine how the proposed dev-mode deep freeze can protect the shared graph without breaking refreshes; done means the chosen behavior and compatibility scope are settled and illegal writes fail at their mutation site.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 32/100