DEV: Ensure `DModal` model argument is still available during destroy (#22411)

Previously, the `@model` argument would be unset before the component's `willDestroy` hook was called. Wrapping up the component and the opts in a single tracked `activeModal` field, and then using the `#each` helper with an array of 1 element means that Glimmer will keep the `@model` argument available until the end of the component's lifecycle.
This commit is contained in:
David Taylor 2023-07-04 14:40:41 +01:00 committed by GitHub
parent 999014e8e5
commit e549b0f132
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 55 additions and 12 deletions

View File

@ -1,11 +1,14 @@
<div class="modal-container" {{did-insert this.modal.setContainerElement}}> <div class="modal-container" {{did-insert this.modal.setContainerElement}}>
</div> </div>
{{#if this.modal.modalBodyComponent}} {{#if this.modal.activeModal}}
<this.modal.modalBodyComponent {{#each (array this.modal.activeModal) as |activeModal|}}
@model={{this.modal.opts.model}} {{! #each ensures that the activeModal component/model are updated atomically }}
<activeModal.component
@model={{activeModal.opts.model}}
@closeModal={{this.closeModal}} @closeModal={{this.closeModal}}
/> />
{{/each}}
{{/if}} {{/if}}
{{! Legacy modals depend on this wrapper being in the DOM at all times. Eventually this will be dropped. {{! Legacy modals depend on this wrapper being in the DOM at all times. Eventually this will be dropped.

View File

@ -20,10 +20,10 @@ const LEGACY_OPTS = new Set([
@disableImplicitInjections @disableImplicitInjections
class ModalService extends Service { class ModalService extends Service {
@tracked modalBodyComponent; @tracked activeModal;
@tracked opts = {}; @tracked opts = {};
@tracked containerElement; @tracked containerElement;
#resolveShowPromise;
@action @action
setContainerElement(element) { setContainerElement(element) {
@ -42,12 +42,13 @@ class ModalService extends Service {
show(modal, opts) { show(modal, opts) {
this.close({ initiatedBy: CLOSE_INITIATED_BY_MODAL_SHOW }); this.close({ initiatedBy: CLOSE_INITIATED_BY_MODAL_SHOW });
let resolveShowPromise;
const promise = new Promise((resolve) => { const promise = new Promise((resolve) => {
this.#resolveShowPromise = resolve; resolveShowPromise = resolve;
}); });
this.opts = opts || {}; this.opts = opts || {};
this.modalBodyComponent = modal; this.activeModal = { component: modal, opts, resolveShowPromise };
const unsupportedOpts = Object.keys(opts).filter((key) => const unsupportedOpts = Object.keys(opts).filter((key) =>
LEGACY_OPTS.has(key) LEGACY_OPTS.has(key)
@ -64,8 +65,8 @@ class ModalService extends Service {
} }
close(data) { close(data) {
this.#resolveShowPromise?.(data); this.activeModal?.resolveShowPromise?.(data);
this.#resolveShowPromise = this.modalBodyComponent = null; this.activeModal = null;
this.opts = {}; this.opts = {};
} }
} }
@ -240,6 +241,6 @@ export default class ModalServiceWithLegacySupport extends ModalService {
} }
get isLegacy() { get isLegacy() {
return this.name && !this.modalBodyComponent; return this.name && !this.activeModal;
} }
} }

View File

@ -109,5 +109,44 @@ acceptance("Modal service: component-based API", function () {
); );
}); });
test("lifecycle hooks and arguments", async function (assert) {
await visit("/");
const events = [];
class ModalWithLifecycleHooks extends MyModalClass {
constructor() {
super(...arguments);
events.push(`constructor: ${this.args.model?.data}`);
}
willDestroy() {
events.push(`willDestroy: ${this.args.model?.data}`);
}
}
const modalService = getOwner(this).lookup("service:modal");
modalService.show(ModalWithLifecycleHooks, {
model: { data: "argumentValue" },
});
await settled();
assert.deepEqual(
events,
["constructor: argumentValue"],
"constructor called with args available"
);
modalService.close();
await settled();
assert.deepEqual(
events,
["constructor: argumentValue", "willDestroy: argumentValue"],
"constructor called with args available"
);
});
// (See also, `tests/integration/component/d-modal-test.js`) // (See also, `tests/integration/component/d-modal-test.js`)
}); });