DEV: Track RenderGlimmer data on a per-key basis

Previously we were tracking the entire `@data` object. That means a change to any argument would cause everything to be invalidated. This commit changes it so that we track on a per-key basis, so that we reduce unnecessary re-rendering.
This commit is contained in:
David Taylor 2024-09-13 17:51:44 +01:00
parent cfbfcc7b81
commit 249900f256
No known key found for this signature in database
GPG Key ID: 46904C18B1D3F434
2 changed files with 51 additions and 9 deletions

View File

@ -1,8 +1,8 @@
import { hasInternalComponentManager } from "@glimmer/manager"; import { hasInternalComponentManager } from "@glimmer/manager";
import { tracked } from "@glimmer/tracking";
import { setComponentTemplate } from "@ember/component"; import { setComponentTemplate } from "@ember/component";
import templateOnly from "@ember/component/template-only"; import templateOnly from "@ember/component/template-only";
import { assert } from "@ember/debug"; import { assert } from "@ember/debug";
import { TrackedObject } from "@ember-compat/tracked-built-ins";
import { createWidgetFrom } from "discourse/widgets/widget"; import { createWidgetFrom } from "discourse/widgets/widget";
const INITIAL_CLASSES = Symbol("RENDER_GLIMMER_INITIAL_CLASSES"); const INITIAL_CLASSES = Symbol("RENDER_GLIMMER_INITIAL_CLASSES");
@ -139,8 +139,22 @@ export default class RenderGlimmer {
} }
this._componentInfo = prev._componentInfo; this._componentInfo = prev._componentInfo;
if (prev.data !== this.data) {
this._componentInfo.data = this.data; const data = this._componentInfo.data;
const incomingData = this.data;
if (incomingData) {
const keysToRemove = new Set(Object.keys(data));
for (let [key, value] of Object.entries(incomingData)) {
if (data[key] !== value) {
data[key] = value;
}
keysToRemove.delete(key);
}
for (const key of keysToRemove) {
delete data[key];
}
} }
return null; return null;
@ -161,7 +175,7 @@ export default class RenderGlimmer {
this._componentInfo = new ComponentInfo({ this._componentInfo = new ComponentInfo({
element, element,
component, component,
data: this.data, data: new TrackedObject(this.data),
setWrapperElementAttrs: (attrs) => setWrapperElementAttrs: (attrs) =>
this.updateElementAttrs(element, attrs), this.updateElementAttrs(element, attrs),
}); });
@ -231,7 +245,7 @@ export function registerWidgetShim(name, tagName, template) {
} }
class ComponentInfo { class ComponentInfo {
@tracked data; data;
element; element;
component; component;
setWrapperElementAttrs; setWrapperElementAttrs;

View File

@ -3,6 +3,7 @@ import templateOnly from "@ember/component/template-only";
import { click, fillIn, render } from "@ember/test-helpers"; import { click, fillIn, render } from "@ember/test-helpers";
import { hbs } from "ember-cli-htmlbars"; import { hbs } from "ember-cli-htmlbars";
import { module, test } from "qunit"; import { module, test } from "qunit";
import DButton from "discourse/components/d-button";
import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { setupRenderingTest } from "discourse/tests/helpers/component-test";
import { exists, query } from "discourse/tests/helpers/qunit-helpers"; import { exists, query } from "discourse/tests/helpers/qunit-helpers";
import widgetHbs from "discourse/widgets/hbs-compiler"; import widgetHbs from "discourse/widgets/hbs-compiler";
@ -61,7 +62,6 @@ class DemoWidget extends Widget {
class DemoComponent extends ClassicComponent { class DemoComponent extends ClassicComponent {
static eventLog = []; static eventLog = [];
classNames = ["demo-component"]; classNames = ["demo-component"];
layout = hbs`<DButton class="component-action-button" @label="component_action" @action={{@action}} /><p class='action-state'>{{@widgetActionTriggered}}</p>`;
init() { init() {
DemoComponent.eventLog.push("init"); DemoComponent.eventLog.push("init");
@ -87,6 +87,22 @@ class DemoComponent extends ClassicComponent {
super.willDestroy(...arguments); super.willDestroy(...arguments);
DemoComponent.eventLog.push("willDestroy"); DemoComponent.eventLog.push("willDestroy");
} }
@bind
logEvent(msg) {
DemoComponent.eventLog.push(msg);
}
<template>
<DButton
class="component-action-button"
@label="component_action"
@action={{@action}}
/>
<p class="action-state">{{@widgetActionTriggered}}</p>
{{this.logEvent "arg1 rendered" @arg1}}
{{this.logEvent "dynamicArg rendered" @dynamicArg}}
</template>
} }
class ToggleDemoWidget extends Widget { class ToggleDemoWidget extends Widget {
@ -207,7 +223,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
assert.deepEqual( assert.deepEqual(
DemoComponent.eventLog, DemoComponent.eventLog,
["init", "didReceiveAttrs", "didInsertElement"], [
"init",
"didReceiveAttrs",
"arg1 rendered",
"dynamicArg rendered",
"didInsertElement",
],
"component is initialized correctly" "component is initialized correctly"
); );
@ -217,7 +239,7 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
await click(".my-widget button"); await click(".my-widget button");
assert.deepEqual( assert.deepEqual(
DemoComponent.eventLog, DemoComponent.eventLog,
["didReceiveAttrs", "didReceiveAttrs"], // once for input, once for event ["didReceiveAttrs", "dynamicArg rendered", "didReceiveAttrs"], // once for input, once for event
"component is notified of attr change during widget rerender" "component is notified of attr change during widget rerender"
); );
@ -235,7 +257,13 @@ module("Integration | Component | Widget | render-glimmer", function (hooks) {
await fillIn("input.dynamic-value-input", "visibleAgain"); await fillIn("input.dynamic-value-input", "visibleAgain");
assert.deepEqual( assert.deepEqual(
DemoComponent.eventLog, DemoComponent.eventLog,
["init", "didReceiveAttrs", "didInsertElement"], [
"init",
"didReceiveAttrs",
"arg1 rendered",
"dynamicArg rendered",
"didInsertElement",
],
"component can be reinitialized" "component can be reinitialized"
); );
}); });