From 6cf716dc817d5e8b14d5e7ab43c10c24f55426b3 Mon Sep 17 00:00:00 2001 From: Wadim Kalmykov <36057469+w-4@users.noreply.github.com> Date: Fri, 16 Oct 2020 04:40:25 +0700 Subject: [PATCH] Fix lifecyle method workarounds (#2378) Essentially, whenever a route is loaded, we add a key to that component. If the key changes, the page completely rerenders. Switching between different routes handled by the same key triggers those rerenders. --- .../core/js/src/common/components/Page.js | 20 +++-------- .../core/js/src/common/utils/mapRoutes.js | 7 ++-- .../js/src/forum/components/DiscussionPage.js | 33 ------------------- .../core/js/src/forum/components/IndexPage.js | 20 ----------- .../core/js/src/forum/components/UserPage.js | 13 -------- 5 files changed, 9 insertions(+), 84 deletions(-) diff --git a/framework/core/js/src/common/components/Page.js b/framework/core/js/src/common/components/Page.js index b0b47d822..f37485cd7 100644 --- a/framework/core/js/src/common/components/Page.js +++ b/framework/core/js/src/common/components/Page.js @@ -10,7 +10,11 @@ export default class Page extends Component { oninit(vnode) { super.oninit(vnode); - this.onNewRoute(); + app.previous = app.current; + app.current = new PageState(this.constructor, { routeName: this.attrs.routeName }); + + app.drawer.hide(); + app.modal.close(); /** * A class name to apply to the body while the route is active. @@ -20,20 +24,6 @@ export default class Page extends Component { this.bodyClass = ''; } - /** - * A collections of actions to run when the route changes. - * This is extracted here, and not hardcoded in oninit, as oninit is not called - * when a different route is handled by the same component, but we still need to - * adjust the current route name. - */ - onNewRoute() { - app.previous = app.current; - app.current = new PageState(this.constructor, { routeName: this.attrs.routeName }); - - app.drawer.hide(); - app.modal.close(); - } - oncreate(vnode) { super.oncreate(vnode); diff --git a/framework/core/js/src/common/utils/mapRoutes.js b/framework/core/js/src/common/utils/mapRoutes.js index f0c96a4ce..ab4f0500d 100644 --- a/framework/core/js/src/common/utils/mapRoutes.js +++ b/framework/core/js/src/common/utils/mapRoutes.js @@ -10,12 +10,13 @@ export default function mapRoutes(routes, basePath = '') { const map = {}; - for (const key in routes) { - const route = routes[key]; + for (const routeName in routes) { + const route = routes[routeName]; map[basePath + route.path] = { render() { - return m(route.component, { routeName: key }); + const key = routeName + JSON.stringify(m.route.param()); + return [m(route.component, { routeName, key })]; }, }; } diff --git a/framework/core/js/src/forum/components/DiscussionPage.js b/framework/core/js/src/forum/components/DiscussionPage.js index 383ad27c2..cf45e1049 100644 --- a/framework/core/js/src/forum/components/DiscussionPage.js +++ b/framework/core/js/src/forum/components/DiscussionPage.js @@ -47,8 +47,6 @@ export default class DiscussionPage extends Page { app.history.push('discussion'); this.bodyClass = 'App--discussion'; - - this.prevRoute = m.route.get(); } onremove() { @@ -96,34 +94,6 @@ export default class DiscussionPage extends Page { ); } - onbeforeupdate(vnode) { - super.onbeforeupdate(vnode); - - if (m.route.get() !== this.prevRoute) { - this.prevRoute = m.route.get(); - - // If we have routed to the same discussion as we were viewing previously, - // cancel the unloading of this controller and instead prompt the post - // stream to jump to the new 'near' param. - if (this.discussion) { - const idParam = m.route.param('id'); - - if (idParam && idParam.split('-')[0] === this.discussion.id()) { - const near = m.route.param('near') || '1'; - - if (near !== String(this.near)) { - this.stream.goToNumber(near); - } - - this.near = near; - } else { - this.onNewRoute(); - this.oninit(vnode); - } - } - } - } - /** * Load the discussion from the API or use the preloaded one. */ @@ -248,10 +218,7 @@ export default class DiscussionPage extends Page { // replace it into the window's history and our own history stack. const url = app.route.discussion(discussion, (this.near = startNumber)); - this.prevRoute = url; - m.route.set(url, null, { replace: true }); window.history.replaceState(null, document.title, url); - app.history.push('discussion', discussion.title()); // If the user hasn't read past here before, then we'll update their read diff --git a/framework/core/js/src/forum/components/IndexPage.js b/framework/core/js/src/forum/components/IndexPage.js index 7995f3565..c94613fba 100644 --- a/framework/core/js/src/forum/components/IndexPage.js +++ b/framework/core/js/src/forum/components/IndexPage.js @@ -42,26 +42,6 @@ export default class IndexPage extends Page { app.history.push('index', app.translator.trans('core.forum.header.back_to_index_tooltip')); this.bodyClass = 'App--index'; - - this.currentPath = m.route.get(); - } - - onbeforeupdate(vnode) { - super.onbeforeupdate(vnode); - - const curPath = m.route.get(); - - if (this.currentPath !== curPath) { - this.onNewRoute(); - - app.discussions.clear(); - - app.discussions.refreshParams(app.search.params()); - - this.currentPath = curPath; - - this.setTitle(); - } } view() { diff --git a/framework/core/js/src/forum/components/UserPage.js b/framework/core/js/src/forum/components/UserPage.js index 1171342f1..cdf56116e 100644 --- a/framework/core/js/src/forum/components/UserPage.js +++ b/framework/core/js/src/forum/components/UserPage.js @@ -27,19 +27,6 @@ export default class UserPage extends Page { this.user = null; this.bodyClass = 'App--user'; - - this.prevUsername = m.route.param('username'); - } - - onbeforeupdate() { - const currUsername = m.route.param('username'); - if (currUsername !== this.prevUsername) { - this.onNewRoute(); - - this.prevUsername = currUsername; - - this.loadUser(currUsername); - } } view() {