Skip to content

Commit 7e9bb9e

Browse files
committed
Revert "fix(gxt): tie classic-tag reactors to instance/owner destroy chains"
This reverts commit ef6ce28.
1 parent ef6ce28 commit 7e9bb9e

3 files changed

Lines changed: 16 additions & 164 deletions

File tree

packages/@ember/-internals/glimmer/lib/renderer.ts

Lines changed: 16 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,90 +1746,6 @@ function intoTarget(into: IntoTarget): Cursor {
17461746
}
17471747
}
17481748

1749-
/**
1750-
* Set of cleanup functions for active GXT renderComponent calls, drained
1751-
* by __gxtCleanupActiveComponents (called from QUnit testStart and from
1752-
* test-case afterEach in internal-test-helpers). Without this, reactors
1753-
* registered by _renderComponentGxt leak across tests because the
1754-
* synthetic `{}` owner default is not in any destroy chain — nobody
1755-
* ever calls destroy() on a freshly-allocated plain object.
1756-
*/
1757-
const _gxtPendingRenderCleanups = new Set<() => void>();
1758-
(globalThis as any).__gxtDrainPendingRenderCleanups = function () {
1759-
if (_gxtPendingRenderCleanups.size === 0) return;
1760-
for (const fn of Array.from(_gxtPendingRenderCleanups)) {
1761-
try {
1762-
fn();
1763-
} catch {
1764-
/* ignore */
1765-
}
1766-
}
1767-
_gxtPendingRenderCleanups.clear();
1768-
};
1769-
1770-
/**
1771-
* Wire `owner` into the destroyable chain so the registered cleanup
1772-
* actually fires on teardown.
1773-
*
1774-
* The renderComponent API documents `owner = {}` as the default. That
1775-
* synthetic plain object isn't in any destroy chain by itself — calling
1776-
* registerDestructor on it records the destructor but it never runs,
1777-
* because nothing ever calls destroy() on the synthetic owner.
1778-
*
1779-
* Three layers of cleanup, all pointing at the same cleanupFn:
1780-
*
1781-
* 1. Ambient parent owner via associateDestroyableChild — when the
1782-
* parent owner is destroyed via the standard glimmer/destroyable
1783-
* chain, the synthetic descends with it.
1784-
* 2. registerDestructor on owner — covers callers that do call
1785-
* destroy(owner) directly.
1786-
* 3. _gxtPendingRenderCleanups Set — eagerly drained by
1787-
* __gxtCleanupActiveComponents at QUnit testStart and from
1788-
* test-case afterEach; this is the only layer that fires
1789-
* synchronously regardless of the runloop, which is what
1790-
* actually prevents the cross-test reactor leak.
1791-
*
1792-
* Idempotent — safe to call from both _renderComponentGxt return paths.
1793-
*/
1794-
function _wireOwnerDestroyChain(owner: object, parentOwner: unknown, cleanupFn: () => void): void {
1795-
// Wrap so each layer fires the underlying cleanup exactly once and
1796-
// removing from the pending Set is automatic.
1797-
let fired = false;
1798-
const oneShot = () => {
1799-
if (fired) return;
1800-
fired = true;
1801-
_gxtPendingRenderCleanups.delete(oneShot);
1802-
try {
1803-
cleanupFn();
1804-
} catch {
1805-
/* ignore */
1806-
}
1807-
};
1808-
_gxtPendingRenderCleanups.add(oneShot);
1809-
if (
1810-
parentOwner &&
1811-
parentOwner !== owner &&
1812-
typeof owner === 'object' &&
1813-
owner !== null &&
1814-
Object.getPrototypeOf(owner) === Object.prototype &&
1815-
!isDestroyed(parentOwner) &&
1816-
!isDestroying(parentOwner) &&
1817-
!isDestroyed(owner) &&
1818-
!isDestroying(owner)
1819-
) {
1820-
try {
1821-
associateDestroyableChild(parentOwner as any, owner as any);
1822-
} catch {
1823-
// parent may not accept association; fall through to register
1824-
}
1825-
}
1826-
try {
1827-
registerDestructor(owner, oneShot);
1828-
} catch {
1829-
// owner may not be destroyable; nothing else to do
1830-
}
1831-
}
1832-
18331749
/**
18341750
* GXT-specific renderComponent implementation.
18351751
* Bypasses Glimmer VM bytecode compilation entirely.
@@ -1863,18 +1779,11 @@ function _renderComponentGxt(
18631779
ensureGxtContext();
18641780

18651781
let destroyed = false;
1866-
let reactorCleanedUp = false;
18671782
let classicReactorUnsub: (() => void) | null = null;
18681783

1869-
// Reactor-only cleanup. Fires when the owner is destroyed via the
1870-
// standard destroyable chain (registered below). Cleaning up the
1871-
// classic-tag reactor is critical to prevent cross-test leaks; the
1872-
// DOM is not touched here because owner destruction does not imply
1873-
// the caller wants the rendered nodes removed (that is what
1874-
// result.destroy() means, see doDestroy).
1875-
const cleanupReactor = () => {
1876-
if (reactorCleanedUp) return;
1877-
reactorCleanedUp = true;
1784+
const doDestroy = () => {
1785+
if (destroyed) return;
1786+
destroyed = true;
18781787
if (classicReactorUnsub) {
18791788
try {
18801789
classicReactorUnsub();
@@ -1883,14 +1792,6 @@ function _renderComponentGxt(
18831792
}
18841793
classicReactorUnsub = null;
18851794
}
1886-
};
1887-
1888-
// Full destroy. Returned to the caller as result.destroy(). Cleans
1889-
// the reactor and wipes the rendered content.
1890-
const doDestroy = () => {
1891-
if (destroyed) return;
1892-
destroyed = true;
1893-
cleanupReactor();
18941795
if (targetElement instanceof Element) {
18951796
targetElement.innerHTML = '';
18961797
}
@@ -1965,7 +1866,12 @@ function _renderComponentGxt(
19651866

19661867
const result: RenderResult = { destroy: doDestroy };
19671868
RENDER_CACHE.set(into, { result, glimmerResult: undefined });
1968-
_wireOwnerDestroyChain(owner, prevOwner, cleanupReactor);
1869+
// Register destructor on owner so owner.destroy() cleans up DOM
1870+
try {
1871+
registerDestructor(owner, doDestroy);
1872+
} catch {
1873+
// owner may not be destroyable
1874+
}
19691875
return result;
19701876
}
19711877

@@ -2277,7 +2183,13 @@ function _renderComponentGxt(
22772183

22782184
const result: RenderResult = { destroy: doDestroy };
22792185
RENDER_CACHE.set(into, { result, glimmerResult: undefined });
2280-
_wireOwnerDestroyChain(owner, prevOwner, cleanupReactor);
2186+
2187+
// Register destructor on owner so that destroying the owner cleans up the DOM
2188+
try {
2189+
registerDestructor(owner, doDestroy);
2190+
} catch {
2191+
// owner may not be destroyable
2192+
}
22812193

22822194
return result;
22832195
}

packages/@ember/-internals/gxt-backend/compile.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5784,16 +5784,6 @@ setInterval(() => {
57845784
if (typeof destroyTracked === 'function') {
57855785
destroyTracked();
57865786
}
5787-
// Eagerly drain any classic-tag reactor cleanups for renderComponent
5788-
// calls whose synthetic plain-object owner ({}) isn't otherwise in a
5789-
// destroy chain (the renderComponent API documents `owner = {}` as the
5790-
// default — see _wireOwnerDestroyChain in renderer.ts). Without this
5791-
// drain, those reactors leak across the QUnit testStart/testDone
5792-
// boundary and clobber shared GXT sync state on subsequent tests.
5793-
const drainPending = (globalThis as any).__gxtDrainPendingRenderCleanups;
5794-
if (typeof drainPending === 'function') {
5795-
drainPending();
5796-
}
57975787
// Reset block params stack
57985788
blockParamsStack.length = 0;
57995789
// Reset current slot params

packages/@ember/-internals/gxt-backend/manager.ts

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4134,26 +4134,6 @@ let _preRerenderSnapshot: Set<any> = new Set();
41344134
}
41354135
}
41364136

4137-
// Phase 0 (eager): drain any classic-tag reactor unsubs registered
4138-
// on instances. These unsubs (set by renderLinkToElement) need to
4139-
// fire SYNCHRONOUSLY here, before Ember's async runloop-based
4140-
// destroy chain runs — otherwise the reactor stays in
4141-
// _classicReactors across the testStart/testDone boundary and
4142-
// fires on the next test's tag dirties. The willDestroy monkey-
4143-
// patch in renderLinkToElement is a fallback for instances not
4144-
// reached by this phase.
4145-
for (const instance of instances) {
4146-
const unsub = instance && instance.__gxtClassicReactorUnsub;
4147-
if (typeof unsub === 'function') {
4148-
try {
4149-
unsub();
4150-
} catch {
4151-
/* ignore */
4152-
}
4153-
instance.__gxtClassicReactorUnsub = null;
4154-
}
4155-
}
4156-
41574137
// Phase 1: willDestroyElement + willClearRender (top-down, element still present)
41584138
for (const instance of instances) {
41594139
try {
@@ -9537,36 +9517,6 @@ function renderLinkToElement(instance: any, args: any, fw: any): HTMLAnchorEleme
95379517
}
95389518
};
95399519
unsub = (_gxtRegisterClassicReactor as any)(wrapped, 'renderLinkToElement');
9540-
// Tie the reactor's lifetime to the LinkTo instance's lifetime.
9541-
// The element-detachment heuristic above is unreliable in routing
9542-
// tests that reattach the same <a> across transitions (counter
9543-
// resets each time); without an additional escape hatch the
9544-
// reactor lives forever and fires across unrelated tests.
9545-
//
9546-
// Two hooks, both pointing at the same unsub:
9547-
//
9548-
// 1. instance.__gxtClassicReactorUnsub — picked up SYNCHRONOUSLY
9549-
// by Phase 0 of __gxtDestroyTrackedInstances. Ember's async
9550-
// runloop-based destroy chain doesn't fire willDestroy soon
9551-
// enough to prevent the reactor from leaking across the
9552-
// testStart/testDone boundary, so we drain it eagerly there.
9553-
// 2. willDestroy override — fallback for instances not reached
9554-
// by __gxtDestroyTrackedInstances (e.g. component destroyed
9555-
// via a route transition rather than test teardown).
9556-
if (instance && typeof instance === 'object' && !instance.__gxtLinkToReactorUnsubInstalled) {
9557-
instance.__gxtClassicReactorUnsub = unsub;
9558-
const prior = instance.willDestroy;
9559-
const priorBound = typeof prior === 'function' ? prior.bind(instance) : null;
9560-
instance.willDestroy = function () {
9561-
try {
9562-
unsub();
9563-
} catch {
9564-
/* ignore */
9565-
}
9566-
if (priorBound) return priorBound();
9567-
};
9568-
instance.__gxtLinkToReactorUnsubInstalled = true;
9569-
}
95709520
};
95719521

95729522
// id attribute (doesn't change over the element's lifetime, so no reactor)

0 commit comments

Comments
 (0)