From 5e244364547516a834517edc9b1ab5d940029f93 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 16 Apr 2020 07:58:04 +0200 Subject: [PATCH] DEV: attempts to fix various leaks (#9428) * DEV: attempts to fix various leaks * scheduleOnce doesnt work with anon function * removes the I18n change --- .../discourse/components/composer-body.js | 4 +- .../discourse/components/composer-editor.js | 10 +-- .../discourse/components/d-editor.js | 6 +- .../edit-category-topic-template.js | 4 +- .../discourse/components/emoji-picker.js | 14 ++-- .../components/expanding-text-area.js | 4 +- .../discourse/components/quote-button.js | 10 ++- .../components/search-advanced-options.js | 2 +- .../discourse/controllers/insert-hyperlink.js | 4 +- .../discourse/controllers/login.js | 5 +- .../discourse/controllers/topic.js | 7 +- .../initializers/keyboard-shortcuts.js | 4 ++ .../initializers/title-notifications.js | 8 +++ .../discourse/lib/keyboard-shortcuts.js | 4 ++ .../discourse/lib/link-mentions.js | 4 +- .../discourse/routes/topic-from-params.js | 4 +- .../javascripts/discourse/routes/topic.js | 70 +++++++++++-------- .../wizard/components/invite-list.js | 4 +- .../wizard/components/wizard-step.js | 6 +- test/javascripts/test_helper.js | 2 + 20 files changed, 103 insertions(+), 73 deletions(-) diff --git a/app/assets/javascripts/discourse/components/composer-body.js b/app/assets/javascripts/discourse/components/composer-body.js index 80bb8d38bb0..e5a6e8261dc 100644 --- a/app/assets/javascripts/discourse/components/composer-body.js +++ b/app/assets/javascripts/discourse/components/composer-body.js @@ -1,7 +1,7 @@ import { run, cancel, - scheduleOnce, + schedule, later, debounce, throttle @@ -65,7 +65,7 @@ export default Component.extend(KeyEnterEscape, { "composer.canEditTopicFeaturedLink" ) resize() { - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { if (!this.element || this.isDestroying || this.isDestroyed) { return; } diff --git a/app/assets/javascripts/discourse/components/composer-editor.js b/app/assets/javascripts/discourse/components/composer-editor.js index b14b0af74ac..e6e000aa09c 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js +++ b/app/assets/javascripts/discourse/components/composer-editor.js @@ -1,4 +1,4 @@ -import { debounce, later, next, scheduleOnce, throttle } from "@ember/runloop"; +import { debounce, later, next, schedule, throttle } from "@ember/runloop"; import Component from "@ember/component"; import userSearch from "discourse/lib/user-search"; import discourseComputed, { @@ -194,7 +194,7 @@ export default Component.extend({ transformComplete: v => v.username || v.name, afterComplete() { // ensures textarea scroll position is correct - scheduleOnce("afterRender", () => $input.blur().focus()); + schedule("afterRender", () => $input.blur().focus()); }, triggerRule: textarea => !inCodeBlock(textarea.value, caretPosition(textarea)) @@ -323,7 +323,7 @@ export default Component.extend({ this.appEvents.on(event, this, this._resetShouldBuildScrollMap); }); - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { $input.on("touchstart mouseenter", () => { if (!$preview.is(":visible")) return; $preview.off("scroll"); @@ -569,7 +569,7 @@ export default Component.extend({ }, _warnMentionedGroups($preview) { - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { var found = this.warnedGroupMentions || []; $preview.find(".mention-group.notify").each((idx, e) => { const $e = $(e); @@ -597,7 +597,7 @@ export default Component.extend({ return; } - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { let found = this.warnedCannotSeeMentions || []; $preview.find(".mention.cannot-see").each((idx, e) => { diff --git a/app/assets/javascripts/discourse/components/d-editor.js b/app/assets/javascripts/discourse/components/d-editor.js index 4dd2c72700a..11ec7b06cad 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js +++ b/app/assets/javascripts/discourse/components/d-editor.js @@ -356,7 +356,7 @@ export default Component.extend({ return; } this.set("preview", cooked); - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { if (this._state !== "inDOM") { return; } @@ -559,7 +559,7 @@ export default Component.extend({ }, _selectText(from, length) { - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { const textarea = this.element.querySelector("textarea.d-editor-input"); const $textarea = $(textarea); const oldScrollPos = $textarea.scrollTop(); @@ -898,7 +898,7 @@ export default Component.extend({ // ensures textarea scroll position is correct _focusTextArea() { const textarea = this.element.querySelector("textarea.d-editor-input"); - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { textarea.blur(); textarea.focus(); }); diff --git a/app/assets/javascripts/discourse/components/edit-category-topic-template.js b/app/assets/javascripts/discourse/components/edit-category-topic-template.js index 9a5d7d7ab1b..1ccf804dd1b 100644 --- a/app/assets/javascripts/discourse/components/edit-category-topic-template.js +++ b/app/assets/javascripts/discourse/components/edit-category-topic-template.js @@ -1,4 +1,4 @@ -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import { buildCategoryPanel } from "discourse/components/edit-category-panel"; import { observes } from "discourse-common/utils/decorators"; @@ -6,7 +6,7 @@ export default buildCategoryPanel("topic-template", { @observes("activeTab") _activeTabChanged: function() { if (this.activeTab) { - scheduleOnce("afterRender", () => + schedule("afterRender", () => this.element.querySelector(".d-editor-input").focus() ); } diff --git a/app/assets/javascripts/discourse/components/emoji-picker.js b/app/assets/javascripts/discourse/components/emoji-picker.js index 88b75993249..90e2fefe13b 100644 --- a/app/assets/javascripts/discourse/components/emoji-picker.js +++ b/app/assets/javascripts/discourse/components/emoji-picker.js @@ -1,5 +1,5 @@ import { inject as service } from "@ember/service"; -import { schedule } from "@ember/runloop"; +import { throttle, debounce, schedule } from "@ember/runloop"; import Component from "@ember/component"; import { on, observes } from "discourse-common/utils/decorators"; import { findRawTemplate } from "discourse/lib/raw-templates"; @@ -12,8 +12,6 @@ import { import { safariHacksDisabled } from "discourse/lib/utilities"; import ENV, { INPUT_DELAY } from "discourse-common/config/environment"; -const { run } = Ember; - const PER_ROW = 11; function customEmojis() { const list = extendedEmojiList(); @@ -54,7 +52,7 @@ export default Component.extend({ recentEmojis: this.emojiStore.favorites }); - run.scheduleOnce("afterRender", this, function() { + schedule("afterRender", this, function() { this._bindEvents(); this._loadCategoriesEmojis(); this._positionPicker(); @@ -110,7 +108,7 @@ export default Component.extend({ filterChanged() { this.$filter.find(".clear-filter").toggle(!_.isEmpty(this.filter)); const filterDelay = this.site.isMobileDevice ? 400 : INPUT_DELAY; - run.debounce(this, this._filterEmojisList, filterDelay); + debounce(this, this._filterEmojisList, filterDelay); }, @observes("selectedDiversity") @@ -309,11 +307,11 @@ export default Component.extend({ _bindResizing() { $(window).on("resize", () => { - run.throttle(this, this._positionPicker, 16); + throttle(this, this._positionPicker, 16); }); $("#reply-control").on("div-resizing", () => { - run.throttle(this, this._positionPicker, 16); + throttle(this, this._positionPicker, 16); }); }, @@ -376,7 +374,7 @@ export default Component.extend({ _bindSectionsScroll() { let onScroll = () => { - run.debounce(this, this._checkVisibleSection, 50); + debounce(this, this._checkVisibleSection, 50); }; this.$list.on("scroll", onScroll); diff --git a/app/assets/javascripts/discourse/components/expanding-text-area.js b/app/assets/javascripts/discourse/components/expanding-text-area.js index ca215cc3f73..94c976c4d33 100644 --- a/app/assets/javascripts/discourse/components/expanding-text-area.js +++ b/app/assets/javascripts/discourse/components/expanding-text-area.js @@ -1,12 +1,12 @@ import { TextArea } from "@ember/component"; -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import { on, observes } from "discourse-common/utils/decorators"; import autosize from "discourse/lib/autosize"; export default TextArea.extend({ @on("didInsertElement") _startWatching() { - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { $(this.element).focus(); autosize(this.element); }); diff --git a/app/assets/javascripts/discourse/components/quote-button.js b/app/assets/javascripts/discourse/components/quote-button.js index 3a352691534..8f7d464543b 100644 --- a/app/assets/javascripts/discourse/components/quote-button.js +++ b/app/assets/javascripts/discourse/components/quote-button.js @@ -1,4 +1,4 @@ -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import Component from "@ember/component"; import discourseDebounce from "discourse/lib/debounce"; import toMarkdown from "discourse/lib/to-markdown"; @@ -138,7 +138,11 @@ export default Component.extend({ } // change the position of the button - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { + if (!this.element || this.isDestroying || this.isDestroyed) { + return; + } + let top = markerOffset.top; let left = markerOffset.left + Math.max(0, parentScrollLeft); @@ -160,6 +164,8 @@ export default Component.extend({ }, didInsertElement() { + this._super(...arguments); + const { isWinphone, isAndroid } = this.capabilities; const wait = isWinphone || isAndroid ? INPUT_DELAY : 25; const onSelectionChanged = discourseDebounce( diff --git a/app/assets/javascripts/discourse/components/search-advanced-options.js b/app/assets/javascripts/discourse/components/search-advanced-options.js index 136c416588e..274e62e7b15 100644 --- a/app/assets/javascripts/discourse/components/search-advanced-options.js +++ b/app/assets/javascripts/discourse/components/search-advanced-options.js @@ -77,7 +77,7 @@ export default Component.extend({ this._init(); - scheduleOnce("afterRender", () => this._update()); + scheduleOnce("afterRender", this, this._update); }, @observes("searchTerm") diff --git a/app/assets/javascripts/discourse/controllers/insert-hyperlink.js b/app/assets/javascripts/discourse/controllers/insert-hyperlink.js index 57d751b56e3..4e5f6ba8214 100644 --- a/app/assets/javascripts/discourse/controllers/insert-hyperlink.js +++ b/app/assets/javascripts/discourse/controllers/insert-hyperlink.js @@ -1,5 +1,5 @@ import { isEmpty } from "@ember/utils"; -import { cancel, debounce, scheduleOnce } from "@ember/runloop"; +import { cancel, debounce, schedule } from "@ember/runloop"; import Controller from "@ember/controller"; import ModalFunctionality from "discourse/mixins/modal-functionality"; import { searchForTerm } from "discourse/lib/search"; @@ -17,7 +17,7 @@ export default Controller.extend(ModalFunctionality, { selectedRow: -1 }); - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { const element = document.querySelector(".insert-link"); element.addEventListener("keydown", e => this.keyDown(e)); diff --git a/app/assets/javascripts/discourse/controllers/login.js b/app/assets/javascripts/discourse/controllers/login.js index de39f7ec8a8..993b1a398f5 100644 --- a/app/assets/javascripts/discourse/controllers/login.js +++ b/app/assets/javascripts/discourse/controllers/login.js @@ -2,8 +2,7 @@ import discourseComputed from "discourse-common/utils/decorators"; import { isEmpty } from "@ember/utils"; import { alias, or, readOnly } from "@ember/object/computed"; import EmberObject from "@ember/object"; -import { next } from "@ember/runloop"; -import { scheduleOnce } from "@ember/runloop"; +import { next, schedule } from "@ember/runloop"; import Controller, { inject as controller } from "@ember/controller"; import { ajax } from "discourse/lib/ajax"; import ModalFunctionality from "discourse/mixins/modal-functionality"; @@ -152,7 +151,7 @@ export default Controller.extend(ModalFunctionality, { // only need to focus the 2FA input for TOTP if (!this.showSecurityKey) { - scheduleOnce("afterRender", () => + schedule("afterRender", () => document .getElementById("second-factor") .querySelector("input") diff --git a/app/assets/javascripts/discourse/controllers/topic.js b/app/assets/javascripts/discourse/controllers/topic.js index 49da4ed9f46..eea22c7d88b 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js +++ b/app/assets/javascripts/discourse/controllers/topic.js @@ -1,8 +1,7 @@ import { isPresent, isEmpty } from "@ember/utils"; import { or, and, not, alias } from "@ember/object/computed"; import EmberObject from "@ember/object"; -import { next } from "@ember/runloop"; -import { scheduleOnce } from "@ember/runloop"; +import { next, schedule } from "@ember/runloop"; import Controller, { inject as controller } from "@ember/controller"; import { bufferedProperty } from "discourse/mixins/buffered-content"; import Composer from "discourse/models/composer"; @@ -137,9 +136,7 @@ export default Controller.extend(bufferedProperty("model"), { return; } - scheduleOnce("afterRender", () => { - this.send("showHistory", post, revision); - }); + schedule("afterRender", () => this.send("showHistory", post, revision)); }, showCategoryChooser: not("model.isPrivateMessage"), diff --git a/app/assets/javascripts/discourse/initializers/keyboard-shortcuts.js b/app/assets/javascripts/discourse/initializers/keyboard-shortcuts.js index a0de2c5d2ad..90860193f90 100644 --- a/app/assets/javascripts/discourse/initializers/keyboard-shortcuts.js +++ b/app/assets/javascripts/discourse/initializers/keyboard-shortcuts.js @@ -7,5 +7,9 @@ export default { initialize(container) { KeyboardShortcuts.init(Mousetrap, container); KeyboardShortcuts.bindEvents(); + }, + + teardown() { + KeyboardShortcuts.teardown(); } }; diff --git a/app/assets/javascripts/discourse/initializers/title-notifications.js b/app/assets/javascripts/discourse/initializers/title-notifications.js index cfc0c3ed6d8..c7ae1d5f06b 100644 --- a/app/assets/javascripts/discourse/initializers/title-notifications.js +++ b/app/assets/javascripts/discourse/initializers/title-notifications.js @@ -13,6 +13,14 @@ export default { .on("notifications:changed", this, "_updateTitle"); }, + teardown(container) { + container + .lookup("service:app-events") + .off("notifications:changed", this, "_updateTitle"); + + this.container = null; + }, + _updateTitle() { const user = this.container.lookup("current-user:main"); if (!user) return; // must be logged in diff --git a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js index db3400cabda..9a3aff88a40 100644 --- a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js +++ b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js @@ -106,6 +106,10 @@ export default { }); }, + teardown() { + this.container = null; + }, + bindKey(key, binding = null) { if (!binding) { binding = DEFAULT_BINDINGS[key]; diff --git a/app/assets/javascripts/discourse/lib/link-mentions.js b/app/assets/javascripts/discourse/lib/link-mentions.js index 30bdcdda95f..02872edaecf 100644 --- a/app/assets/javascripts/discourse/lib/link-mentions.js +++ b/app/assets/javascripts/discourse/lib/link-mentions.js @@ -1,4 +1,4 @@ -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import { ajax } from "discourse/lib/ajax"; import { userPath } from "discourse/lib/url"; import { formatUsername } from "discourse/lib/utilities"; @@ -40,7 +40,7 @@ const checked = {}; const cannotSee = []; function updateFound($mentions, usernames) { - scheduleOnce("afterRender", function() { + schedule("afterRender", function() { $mentions.each((i, e) => { const $e = $(e); const username = usernames[i]; diff --git a/app/assets/javascripts/discourse/routes/topic-from-params.js b/app/assets/javascripts/discourse/routes/topic-from-params.js index 1429fb0c163..bcf904ad38f 100644 --- a/app/assets/javascripts/discourse/routes/topic-from-params.js +++ b/app/assets/javascripts/discourse/routes/topic-from-params.js @@ -1,5 +1,5 @@ import { isEmpty } from "@ember/utils"; -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import DiscourseRoute from "discourse/routes/discourse"; import DiscourseURL from "discourse/lib/url"; import Draft from "discourse/models/draft"; @@ -60,7 +60,7 @@ export default DiscourseRoute.extend({ topicController.subscribe(); // Highlight our post after the next render - scheduleOnce("afterRender", () => + schedule("afterRender", () => this.appEvents.trigger("post:highlight", closest) ); diff --git a/app/assets/javascripts/discourse/routes/topic.js b/app/assets/javascripts/discourse/routes/topic.js index 5aebefe6a43..46a1b3ada91 100644 --- a/app/assets/javascripts/discourse/routes/topic.js +++ b/app/assets/javascripts/discourse/routes/topic.js @@ -1,20 +1,26 @@ import { get } from "@ember/object"; import { isEmpty } from "@ember/utils"; -import { cancel, later, scheduleOnce } from "@ember/runloop"; +import { cancel, later, schedule } from "@ember/runloop"; import DiscourseRoute from "discourse/routes/discourse"; import DiscourseURL from "discourse/lib/url"; import { ID_CONSTRAINT } from "discourse/models/topic"; import { EventTarget } from "rsvp"; -let isTransitioning = false, - scheduledReplace = null, - lastScrollPos = null; - const SCROLL_DELAY = 500; import showModal from "discourse/lib/show-modal"; const TopicRoute = DiscourseRoute.extend({ + init() { + this._super(...arguments); + + this.setProperties({ + isTransitioning: false, + scheduledReplace: null, + lastScrollPos: null + }); + }, + redirect() { return this.redirectIfLoginRequired(); }, @@ -169,7 +175,7 @@ const TopicRoute = DiscourseRoute.extend({ // Use replaceState to update the URL once it changes postChangedRoute(currentPost) { // do nothing if we are transitioning to another route - if (isTransitioning || TopicRoute.disableReplaceState) { + if (this.isTransitioning || TopicRoute.disableReplaceState) { return; } @@ -180,14 +186,17 @@ const TopicRoute = DiscourseRoute.extend({ postUrl += "/" + currentPost; } - cancel(scheduledReplace); - lastScrollPos = parseInt($(document).scrollTop(), 10); - scheduledReplace = later( - this, - "_replaceUnlessScrolling", - postUrl, - Ember.Test ? 0 : SCROLL_DELAY - ); + cancel(this.scheduledReplace); + + this.setProperties({ + lastScrollPos: parseInt($(document).scrollTop(), 10), + scheduledReplace: later( + this, + "_replaceUnlessScrolling", + postUrl, + Ember.Test ? 0 : SCROLL_DELAY + ) + }); } }, @@ -198,8 +207,8 @@ const TopicRoute = DiscourseRoute.extend({ willTransition() { this._super(...arguments); - cancel(scheduledReplace); - isTransitioning = true; + cancel(this.scheduledReplace); + this.set("isTransitioning", true); return true; } }, @@ -208,17 +217,20 @@ const TopicRoute = DiscourseRoute.extend({ // within a topic until scrolling stops _replaceUnlessScrolling(url) { const currentPos = parseInt($(document).scrollTop(), 10); - if (currentPos === lastScrollPos) { + if (currentPos === this.lastScrollPos) { DiscourseURL.replaceState(url); return; } - lastScrollPos = currentPos; - scheduledReplace = later( - this, - "_replaceUnlessScrolling", - url, - SCROLL_DELAY - ); + + this.setProperties({ + lastScrollPos: currentPos, + scheduledReplace: later( + this, + "_replaceUnlessScrolling", + url, + SCROLL_DELAY + ) + }); }, setupParams(topic, params) { @@ -264,7 +276,7 @@ const TopicRoute = DiscourseRoute.extend({ activate() { this._super(...arguments); - isTransitioning = false; + this.set("isTransitioning", false); const topic = this.modelFor("topic"); this.session.set("lastTopicIdViewed", parseInt(topic.get("id"), 10)); @@ -292,7 +304,7 @@ const TopicRoute = DiscourseRoute.extend({ setupController(controller, model) { // In case we navigate from one topic directly to another - isTransitioning = false; + this.set("isTransitioning", false); controller.setProperties({ model, @@ -314,9 +326,9 @@ const TopicRoute = DiscourseRoute.extend({ // We reset screen tracking every time a topic is entered this.screenTrack.start(model.get("id"), controller); - scheduleOnce("afterRender", () => { - this.appEvents.trigger("header:update-topic", model); - }); + schedule("afterRender", () => + this.appEvents.trigger("header:update-topic", model) + ); } }); diff --git a/app/assets/javascripts/wizard/components/invite-list.js b/app/assets/javascripts/wizard/components/invite-list.js index 2c541d38c5d..39435e0f894 100644 --- a/app/assets/javascripts/wizard/components/invite-list.js +++ b/app/assets/javascripts/wizard/components/invite-list.js @@ -1,4 +1,4 @@ -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import Component from "@ember/component"; export default Component.extend({ @@ -63,7 +63,7 @@ export default Component.extend({ this.updateField(); this.set("inviteEmail", ""); - scheduleOnce("afterRender", () => + schedule("afterRender", () => this.element.querySelector(".invite-email").focus() ); }, diff --git a/app/assets/javascripts/wizard/components/wizard-step.js b/app/assets/javascripts/wizard/components/wizard-step.js index dd36c588914..9056e9a0520 100644 --- a/app/assets/javascripts/wizard/components/wizard-step.js +++ b/app/assets/javascripts/wizard/components/wizard-step.js @@ -1,4 +1,4 @@ -import { scheduleOnce } from "@ember/runloop"; +import { schedule } from "@ember/runloop"; import Component from "@ember/component"; import getUrl from "discourse-common/lib/get-url"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; @@ -96,7 +96,7 @@ export default Component.extend({ }, autoFocus() { - scheduleOnce("afterRender", () => { + schedule("afterRender", () => { const $invalid = $(".wizard-field.invalid:eq(0) .wizard-focusable"); if ($invalid.length) { @@ -108,7 +108,7 @@ export default Component.extend({ }, animateInvalidFields() { - scheduleOnce("afterRender", () => + schedule("afterRender", () => $(".invalid input[type=text], .invalid textarea").wiggle(2, 100) ); }, diff --git a/test/javascripts/test_helper.js b/test/javascripts/test_helper.js index 0f2fd5ec0a4..38afc19d67c 100644 --- a/test/javascripts/test_helper.js +++ b/test/javascripts/test_helper.js @@ -201,6 +201,8 @@ QUnit.testDone(function() { }); window.MessageBus.unsubscribe("*"); + delete window.server; + window.Mousetrap.reset(); }); // Load ES6 tests