From e08a0b509dc92a95c289d12f2475189ee378f93f Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 3 Mar 2023 11:48:58 +0000 Subject: [PATCH] DEV: Support `@debounce` decorator in native class syntax (#20521) The implementation previously generated a descriptor with an `initializer()`, and bound the function to the `this` context of the initializer. In native class syntax, the initializer of a descriptor is only called once, with a `this` context of the constructor, not the instance. This commit updates the implementation so that it generates the bound function on-demand using a getter. This is the same strategy employed by ember's built-in `@action` decorator. Unfortunately, this use of a getter means that the `@observes` decorator does not support being directly chained to `@debounce`. It throws the error "`observer must be provided a function or an observer definition`". The workaround is to put the observer on its own function, which then calls the debounced function. Given that we're aiming to reduce our usage of `@observes`, we've accepted the need for this workaround rather than spending the time to patch the implementation of `@observes`. --- .../addon/controllers/admin-site-settings.js | 4 ++ .../addon/utils/decorators.js | 12 +++-- .../discourse/app/controllers/group-index.js | 4 ++ .../app/controllers/group-requests.js | 4 ++ .../app/controllers/user-invited-show.js | 4 ++ .../tests/unit/utils/decorators-test.js | 52 ++++++++++++++++--- .../discourse-local-dates-create-form.js | 4 ++ 7 files changed, 73 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js b/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js index 80355cddc7b..1e61c2c85a4 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js @@ -112,6 +112,10 @@ export default Controller.extend({ }, @observes("filter", "onlyOverridden", "model") + optsChanged() { + this.filterContent(); + }, + @debounce(INPUT_DELAY) filterContent() { if (this._skipBounce) { diff --git a/app/assets/javascripts/discourse-common/addon/utils/decorators.js b/app/assets/javascripts/discourse-common/addon/utils/decorators.js index e4cd53ca62e..a6519aab200 100644 --- a/app/assets/javascripts/discourse-common/addon/utils/decorators.js +++ b/app/assets/javascripts/discourse-common/addon/utils/decorators.js @@ -100,10 +100,9 @@ export function debounce(delay, immediate = false) { return { enumerable: descriptor.enumerable, configurable: descriptor.configurable, - writable: descriptor.writable, - initializer() { + get: function () { const originalFunction = descriptor.value; - const debounced = function (...args) { + const debounced = (...args) => { return discourseDebounce( this, originalFunction, @@ -113,6 +112,13 @@ export function debounce(delay, immediate = false) { ); }; + // Memoize on instance for future access + Object.defineProperty(this, name, { + value: debounced, + enumerable: descriptor.enumerable, + configurable: descriptor.configurable, + }); + return debounced; }, }; diff --git a/app/assets/javascripts/discourse/app/controllers/group-index.js b/app/assets/javascripts/discourse/app/controllers/group-index.js index a1fa8632995..55dc4fe1f31 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-index.js +++ b/app/assets/javascripts/discourse/app/controllers/group-index.js @@ -25,6 +25,10 @@ export default Controller.extend({ bulkSelection: null, @observes("filterInput") + filterInputChanged() { + this._setFilter(); + }, + @debounce(500) _setFilter() { this.set("filter", this.filterInput); diff --git a/app/assets/javascripts/discourse/app/controllers/group-requests.js b/app/assets/javascripts/discourse/app/controllers/group-requests.js index 71f0d0c3271..d8d87662cdd 100644 --- a/app/assets/javascripts/discourse/app/controllers/group-requests.js +++ b/app/assets/javascripts/discourse/app/controllers/group-requests.js @@ -19,6 +19,10 @@ export default Controller.extend({ loading: false, @observes("filterInput") + filterInputChanged() { + this._setFilter(); + }, + @debounce(500) _setFilter() { this.set("filter", this.filterInput); diff --git a/app/assets/javascripts/discourse/app/controllers/user-invited-show.js b/app/assets/javascripts/discourse/app/controllers/user-invited-show.js index 14e8bc968f9..603306a5594 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-invited-show.js +++ b/app/assets/javascripts/discourse/app/controllers/user-invited-show.js @@ -30,6 +30,10 @@ export default Controller.extend({ }, @observes("searchTerm") + searchTermChanged() { + this._searchTermChanged(); + }, + @debounce(INPUT_DELAY) _searchTermChanged() { Invite.findInvitedBy(this.user, this.filter, this.searchTerm).then( diff --git a/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js b/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js index 4426bd29166..0db211a87b6 100644 --- a/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js +++ b/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js @@ -8,6 +8,7 @@ import discourseComputed, { observes, on, } from "discourse-common/utils/decorators"; +import { observes as nativeClassObserves } from "@ember-decorators/object"; import { exists } from "discourse/tests/helpers/qunit-helpers"; import { hbs } from "ember-cli-htmlbars"; import EmberObject from "@ember/object"; @@ -71,15 +72,43 @@ const TestStub = EmberObject.extend({ this.state = state; }, - // Note: it only works in this particular order: - // `@observes()` first, then `@debounce()` @observes("prop") + propChanged() { + this.react(); + }, + @debounce(50) react() { this.otherCounter++; }, }); +const ClassSyntaxTestStub = class extends EmberObject { + counter = 0; + otherCounter = 0; + state = null; + + @debounce(50) + increment(value) { + this.counter += value; + } + + @debounce(50, true) + setState(state) { + this.state = state; + } + + @nativeClassObserves("prop") + propChanged() { + this.react(); + } + + @debounce(50) + react() { + this.otherCounter++; + } +}; + module("Unit | Utils | decorators", function (hooks) { setupRenderingTest(hooks); @@ -167,15 +196,22 @@ module("Unit | Utils | decorators", function (hooks) { assert.strictEqual(stub.state, "foo"); }); - test("debounce works with @observe", async function (assert) { - const stub = TestStub.create(); + test("debounce works with native class syntax", async function (assert) { + const stub = ClassSyntaxTestStub.create(); - stub.set("prop", 1); - stub.set("prop", 2); - stub.set("prop", 3); + stub.increment(1); + stub.increment(1); + stub.increment(1); await settled(); - assert.strictEqual(stub.otherCounter, 1); + assert.strictEqual(stub.counter, 1); + + stub.increment(500); + stub.increment(1000); + stub.increment(5); + await settled(); + + assert.strictEqual(stub.counter, 6); }); test("@observes works via .extend and native class syntax", async function (assert) { diff --git a/plugins/discourse-local-dates/assets/javascripts/discourse/components/discourse-local-dates-create-form.js b/plugins/discourse-local-dates/assets/javascripts/discourse/components/discourse-local-dates-create-form.js index dfc52ccc7bc..48fc3f619e6 100644 --- a/plugins/discourse-local-dates/assets/javascripts/discourse/components/discourse-local-dates-create-form.js +++ b/plugins/discourse-local-dates/assets/javascripts/discourse/components/discourse-local-dates-create-form.js @@ -61,6 +61,10 @@ export default Component.extend({ }, @observes("computedConfig.{from,to,options}", "options", "isValid", "isRange") + configChanged() { + this._renderPreview(); + }, + @debounce(INPUT_DELAY) async _renderPreview() { if (this.markup) {