diff --git a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts index c4f6909a579..4529a5364f5 100644 --- a/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/syntax/outlet.ts @@ -149,15 +149,29 @@ export const outletHelper = internalHelper( // Store the value of the model let model = valueForRef(modelRef); + // The controller for this outlet, used to verify the outletRef + // still points to the correct route's data. + let outletController = state.controller; + // Create a compute ref which we pass in as the `{{@model}}` reference // for the outlet. This ref will update and return the value of the // model _until_ the outlet itself changes. Once the outlet changes, // dynamic scope also changes, and so the original model ref would not // provide the correct updated value. So we stop updating and return // the _last_ model value for that outlet. + // + // We also verify that the outletRef still resolves to this route's + // data by comparing controller identity. This handles the case where + // a parent outlet is torn down first: the dynamic scope refs now + // point to the new route's outlet state, but this outlet's outer + // compute ref hasn't re-evaluated yet, so `lastState === state` is + // still true. The controller check catches this case. named['model'] = createComputeRef(() => { if (lastState === state) { - model = valueForRef(modelRef); + let currentOutlet = valueForRef(outletRef); + if (currentOutlet?.render?.controller === outletController) { + model = valueForRef(modelRef); + } } return model; diff --git a/smoke-tests/scenarios/basic-test.ts b/smoke-tests/scenarios/basic-test.ts index 1e575338e0e..a03c35a689d 100644 --- a/smoke-tests/scenarios/basic-test.ts +++ b/smoke-tests/scenarios/basic-test.ts @@ -18,10 +18,39 @@ function basicTest(scenarios: Scenarios, appName: string) { } Router.map(function () { - this.route('example-gjs-route') + this.route('example-gjs-route'); + this.route('a', function () { + this.route('b'); + this.route('c'); + }); + this.route('d', function () { + this.route('e'); + }); + this.route('f'); + this.route('item', { path: '/item/:item_id' }); + this.route('g', function () { + this.route('h', function () { + this.route('i'); + }); + }); }); `, components: { + 'model-probe.gjs': ` + import Component from '@glimmer/component'; + + const destroyedModels = []; + export function getDestroyedModels() { return destroyedModels; } + export function clearDestroyedModels() { destroyedModels.length = 0; } + + export default class ModelProbe extends Component { + willDestroy() { + super.willDestroy(); + destroyedModels.push(this.args.model); + } + + } + `, 'interactive-example.js': ` import { template } from '@ember/template-compiler'; import Component from '@glimmer/component'; @@ -66,6 +95,54 @@ function basicTest(scenarios: Scenarios, appName: string) { } } `, + 'a.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'a'; } } + `, + a: { + 'b.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'b'; } } + `, + 'c.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'c'; } } + `, + }, + 'd.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'd'; } } + `, + d: { + 'e.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'e'; } } + `, + }, + 'f.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'f'; } } + `, + 'item.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model(params) { return params.item_id; } } + `, + 'g.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'g'; } } + `, + g: { + 'h.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'h'; } } + `, + h: { + 'i.js': ` + import Route from '@ember/routing/route'; + export default class extends Route { model() { return 'i'; } } + `, + }, + }, }, templates: { 'example-gjs-route.gjs': ` @@ -83,6 +160,27 @@ function basicTest(scenarios: Scenarios, appName: string) { } `, + 'a.gjs': ``, + a: { + 'b.gjs': ` + import ModelProbe from '${appName}/components/model-probe'; + + `, + }, + 'item.gjs': ` + import ModelProbe from '${appName}/components/model-probe'; + + `, + 'g.gjs': ``, + g: { + 'h.gjs': ``, + h: { + 'i.gjs': ` + import ModelProbe from '${appName}/components/model-probe'; + + `, + }, + }, }, }, tests: { @@ -104,6 +202,71 @@ function basicTest(scenarios: Scenarios, appName: string) { }); }); `, + 'model-stability-test.js': ` + import { module, test } from 'qunit'; + import { visit } from '@ember/test-helpers'; + import { setupApplicationTest } from '${appName}/tests/helpers'; + import { getDestroyedModels, clearDestroyedModels } from '${appName}/components/model-probe'; + + module('Acceptance | @model stability during route transitions', function (hooks) { + setupApplicationTest(hooks); + hooks.beforeEach(function () { clearDestroyedModels(); }); + + test('@model should be stable when transitioning out of the route', async function (assert) { + await visit('/a/b'); + await visit('/a'); + + await visit('/a/b'); + await visit('/a/c'); + + await visit('/a/b'); + await visit('/d'); + + await visit('/a/b'); + await visit('/d/e'); + + await visit('/a/b'); + await visit('/f'); + + assert.deepEqual( + getDestroyedModels(), + ['b', 'b', 'b', 'b', 'b'], + 'The @model value should remain stable in willDestroy for all transition types' + ); + }); + + test('@model should update when the model changes on the same route', async function (assert) { + await visit('/item/first'); + assert.dom().containsText('first'); + + await visit('/item/second'); + assert.dom().containsText('second'); + + await visit('/item/third'); + assert.dom().containsText('third'); + + // Leave the route entirely — the destroyed model should be the latest one + await visit('/f'); + + assert.deepEqual( + getDestroyedModels(), + ['third'], + 'The @model value should be the latest model when finally destroyed' + ); + }); + + test('@model should be stable when grandparent outlet tears down', async function (assert) { + await visit('/g/h/i'); + await visit('/f'); + + assert.deepEqual( + getDestroyedModels(), + ['i'], + 'The @model value should remain stable when grandparent outlet tears down' + ); + }); + }); + `, }, integration: { 'tracked-built-ins-macro-test.gjs': `