MemberJunction / MemberJunction/MJ
GetGlobalObjectStore throws and catches a ReferenceError on every call in Node (~1.4µs), taxing every BaseSingleton .Instance access
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 323
Description
## Summary
`GetGlobalObjectStore()` ([`packages/MJGlobal/src/util.ts:18`](https://github.com/MemberJunction/MJ/blob/next/packages/MJGlobal/src/util.ts#L18)) probes a **bare `window` identifier** inside a `try`. In Node that is not a falsy read — it throws a `ReferenceError`, which the `catch` then swallows before falling back to `global`.
```ts
export function GetGlobalObjectStore(): GlobalObjectStore | null {
try {
if (window) // ← ReferenceError in Node, on EVERY call
return window as unknown as GlobalObjectStore;
else {
if (global) return global as unknown as GlobalObjectStore;
else return null;
}
}
catch (e) {
try {
if (global) return global as unknown as GlobalObjectStore;
...
```
Throwing and catching an exception costs roughly **45x** the property reads it is guarding, and this function sits underneath `BaseSingleton.getInstance()` — so **every `.Instance` access anywhere in MJ pays it, on every request**, server-side.
## Measurements
Node v24.13.0, darwin-arm64, microbenchmarks against the built `dist`:
| Call | ns/op |
|---|---|
| `GetGlobalObjectStore()` — current implementation | **1,434** |
| Same semantics with a `typeof` guard (no throw) | **32** |
| → the exception's share | **97.8%** |
| `LocalCacheManager.Instance` (a single `BaseSingleton.getInstance`) | **1,983** |
| `GetDataHooks('PostRunView')` with 0 hooks registered | **1,645** |
For scale: a 383-row `LocalCacheManager.GetRunViewResult()` cache hit measures 2,534 ns, so a *single* `.Instance` access costs about 78% of the cache lookup it is usually helping perform.
## Scope
- `BaseSingleton.getInstance()` and `BaseSingleton.GetGlobalObjectStore()` ([`packages/MJGlobal/src/BaseSingleton.ts:16,39,51`](https://github.com/MemberJunction/MJ/blob/next/packages/MJGlobal/src/BaseSingleton.ts)) — so every MJ singleton access
- 83 files reference `GetGlobalObjectStore` directly
**Browsers are unaffected** — `window` exists there, so the throw path is Node-only. This is purely a server-side cost.
## Proposed fix
Replace the exception-driven probe with an explicit existence check. `globalThis` is available in every environment MJ targets (Node 12+, all modern browsers) and removes the branching entirely:
```ts
export function GetGlobalObjectStore(): GlobalObjectStore | null {
if (typeof globalThis !== 'undefined' && globalThis) {
return globalThis as unknown as GlobalObjectStore;
}
return null;
}
```
If preserving the exact `window`-before-`global` preference matters (it should not — in a browser `globalThis === window`, and in Node `globalThis === global`), the conservative variant is:
```ts
if (typeof window !== 'undefined' && window) return window as unknown as GlobalObjectStore;
if (typeof global !== 'undefined' && global) return global as unknown as GlobalObjectStore;
return null;
```
Both are semantically identical to today's behavior in every environment; only the cost differs.
## Risk
Low but broad. The function backs the Global Object Store that guarantees singleton identity across duplicated module copies, so the change should be verified to return the *same object identity* as before in Node, browser, and Vitest environments — a returned-identity test plus the existing `BaseSingleton` suites should cover it.
## Context
Found while measuring the cost of running `PostRunView` data hooks on cache-hit paths in #3425. That PR works around the cost locally by memoizing the resolved store inside `dataHooks.ts`; this issue is the source fix, deliberately kept out of that PR because the blast radius is repo-wide.
Contributor guide
Research direction
Start with packages/MJGlobal/src/util.ts and packages/MJGlobal/src/BaseSingleton.ts, focusing on GetGlobalObjectStore and BaseSingleton.getInstance. Verify the returned global-store identity in Node, browser, and Vitest environments, then run the existing BaseSingleton suites. Done means the lookup preserves singleton identity without the Node exception path and the suites remain passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, performance
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100