[BUGFIX] Ensure QP definitions interop with tracked props

emberjs · Sep 5, 2019 · 529feee · 529feee
1 parent 7d125b7
commit 529feee
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 3 deletions.
diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts
@@ -56,7 +56,13 @@ export { default as getProperties } from './lib/get_properties';
 export { default as setProperties } from './lib/set_properties';
 export { default as expandProperties } from './lib/expand_properties';
 
-export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
+export {
+  addObserver,
+  activateObserver,
+  removeObserver,
+  flushAsyncObservers,
+  flushSyncObservers,
+} from './lib/observer';
 export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
 export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
 export { tagForProperty, tagFor, markObjectAsDirty, UNKNOWN_PROPERTY_TAG } from './lib/tags';

diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts
@@ -2,6 +2,7 @@ import { OwnedTemplate, TemplateFactory } from '@ember/-internals/glimmer';
 import {
   computed,
   defineProperty,
+  descriptorForProperty,
   get,
   getProperties,
   isEmpty,
@@ -17,9 +18,11 @@ import {
   setFrameworkClass,
   typeOf,
 } from '@ember/-internals/runtime';
+import { lookupDescriptor } from '@ember/-internals/utils';
 import Controller from '@ember/controller';
 import { assert, deprecate, info, isTesting } from '@ember/debug';
 import { ROUTER_EVENTS } from '@ember/deprecated-features';
+import { dependentKeyCompat } from '@ember/object/compat';
 import { assign } from '@ember/polyfills';
 import { once } from '@ember/runloop';
 import { classify } from '@ember/string';
@@ -2005,6 +2008,21 @@ function mergeEachQueryParams(controllerQP: {}, routeQP: {}) {
 
 function addQueryParamsObservers(controller: any, propNames: string[]) {
   propNames.forEach(prop => {
+    if (descriptorForProperty(controller, prop) === undefined) {
+      let desc = lookupDescriptor(controller, prop);
+
+      if (desc !== null && (typeof desc.get === 'function' || typeof desc.set === 'function')) {
+        defineProperty(
+          controller,
+          prop,
+          dependentKeyCompat({
+            get: desc.get,
+            set: desc.set,
+          })
+        );
+      }
+    }
+
     controller.addObserver(`${prop}.[]`, controller, controller._qpChanged);
   });
 }

diff --git a/packages/@ember/runloop/index.js b/packages/@ember/runloop/index.js
@@ -1,6 +1,6 @@
 import { assert } from '@ember/debug';
 import { onErrorTarget } from '@ember/-internals/error-handling';
-import { flushAsyncObservers } from '@ember/-internals/metal';
+import { flushAsyncObservers, flushSyncObservers } from '@ember/-internals/metal';
 import Backburner from 'backburner';
 import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features';
 
@@ -17,6 +17,7 @@ function onEnd(current, next) {
   currentRunLoop = next;
 
   if (EMBER_METAL_TRACKED_PROPERTIES) {
+    flushSyncObservers();
     flushAsyncObservers();
   }
 }
@@ -26,6 +27,7 @@ let flush;
 if (EMBER_METAL_TRACKED_PROPERTIES) {
   flush = function(queueName, next) {
     if (queueName === 'render' || queueName === _rsvpErrorQueue) {
+      flushSyncObservers();
       flushAsyncObservers();
     }
 

diff --git a/packages/ember/tests/routing/query_params_test.js b/packages/ember/tests/routing/query_params_test.js
@@ -3,7 +3,7 @@ import { dasherize } from '@ember/string';
 import { RSVP, Object as EmberObject, A as emberA } from '@ember/-internals/runtime';
 import { run } from '@ember/runloop';
 import { peekMeta } from '@ember/-internals/meta';
-import { get, computed } from '@ember/-internals/metal';
+import { get, computed, tracked } from '@ember/-internals/metal';
 import { Route } from '@ember/-internals/routing';
 import { PARAMS_SYMBOL } from 'router_js';
 
@@ -1581,5 +1581,71 @@ moduleFor(
       let controller = this.getController('constructor');
       assert.equal(get(controller, 'foo'), '999');
     }
+
+    async ['@test Single query params defined with tracked properties can be on the controller and reflected in the url'](
+      assert
+    ) {
+      assert.expect(3);
+
+      this.router.map(function() {
+        this.route('home', { path: '/' });
+      });
+
+      this.add(
+        `controller:home`,
+        Controller.extend({
+          queryParams: ['foo'],
+          foo: tracked(),
+        })
+      );
+
+      await this.visitAndAssert('/');
+      let controller = this.getController('home');
+
+      controller.foo = '456';
+      await runLoopSettled();
+      this.assertCurrentPath('/?foo=456');
+
+      controller.foo = '987';
+      await runLoopSettled();
+      this.assertCurrentPath('/?foo=987');
+    }
+
+    async ['@test Single query params defined with native getters and tracked properties can be on the controller and reflected in the url'](
+      assert
+    ) {
+      assert.expect(3);
+
+      this.router.map(function() {
+        this.route('home', { path: '/' });
+      });
+
+      this.add(
+        `controller:home`,
+        Controller.extend({
+          queryParams: ['foo'],
+          get foo() {
+            return this.bar;
+          },
+
+          set foo(value) {
+            this.bar = value;
+          },
+
+          bar: tracked(),
+        })
+      );
+
+      await this.visitAndAssert('/');
+      let controller = this.getController('home');
+
+      controller.bar = '456';
+      await runLoopSettled();
+      this.assertCurrentPath('/?foo=456');
+
+      controller.bar = '987';
+      await runLoopSettled();
+      this.assertCurrentPath('/?foo=987');
+    }
   }
 );