From 8c567f53acf0921dfcf73c4f34e44214ba46f98a Mon Sep 17 00:00:00 2001 From: Alon Bukai Date: Thu, 3 Oct 2019 11:36:19 +0300 Subject: [PATCH 1/4] tests: Add failing test for QP helper with let Using (query-params) helper outside of a link-to is incorrectly failing --- .../components/link-to/query-params-curly-test.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js index dd190616e22..3dd3bc7ec24 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/link-to/query-params-curly-test.js @@ -76,13 +76,16 @@ moduleFor( this.addTemplate( 'index', - `{{#let (query-params foo='456' bar='NAW') as |qp|}}{{link-to 'Index' 'index' qp}}{{/let}}` + `{{#let (query-params foo='456' alon='BUKAI') as |qp|}}{{link-to 'Index' 'index' qp}}{{/let}}` ); - return assert.rejectsAssertion( - this.visit('/'), - /The `\(query-params\)` helper can only be used when invoking the `{{link-to}}` component\./ - ); + return this.visit('/').then(() => { + this.assertComponentElement(this.firstChild, { + tagName: 'a', + attrs: { href: '/?alon=BUKAI&foo=456', class: classMatcher('ember-view') }, + content: 'Index', + }); + }); } } ); From 4dc4c536867a1a60a8af9c29bf535bb5deac450f Mon Sep 17 00:00:00 2001 From: Alon Bukai Date: Thu, 3 Oct 2019 11:37:41 +0300 Subject: [PATCH 2/4] fix: Fixed link-to assert with (qp) helper Using (query-params) helper outside of link-to should not throw --- .../-internals/glimmer/lib/components/link-to.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index 710ce259959..e57bddd465b 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -838,14 +838,14 @@ const LinkComponent = EmberComponent.extend({ ) ); - if (DEBUG && this.query === UNDEFINED) { - let { _models: models } = this; - let lastModel = models.length > 0 && models[models.length - 1]; - - assert( - 'The `(query-params)` helper can only be used when invoking the `{{link-to}}` component.', - !(lastModel && lastModel.isQueryParams) - ); + let { _models: models } = this; + if (models.length > 0) { + let lastModel = models[models.length - 1]; + + if (typeof lastModel === 'object' && lastModel !== null && lastModel.isQueryParams) { + this.query = lastModel.values; + models.pop(); + } } return; From fc7cb2d8ef082d3e691107d401a1a353f27f7b9b Mon Sep 17 00:00:00 2001 From: Alon Bukai Date: Fri, 4 Oct 2019 16:44:13 +0300 Subject: [PATCH 3/4] feat: Assert "Cannot pass QPs object in @models and @query at same time --- .../glimmer/lib/components/link-to.ts | 20 +++++++++++++------ .../routing/lib/system/query_params.ts | 8 +++++++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index e57bddd465b..ef724e43a61 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -3,6 +3,7 @@ */ import { alias, computed } from '@ember/-internals/metal'; +import { isQueryParams } from '@ember/-internals/routing/lib/system/query_params'; import { isSimpleClick } from '@ember/-internals/views'; import { assert, warn } from '@ember/debug'; import { flaggedInstrument } from '@ember/instrumentation'; @@ -838,14 +839,21 @@ const LinkComponent = EmberComponent.extend({ ) ); - let { _models: models } = this; - if (models.length > 0) { - let lastModel = models[models.length - 1]; + if (this.query === UNDEFINED) { + let { _models: models } = this; + if (models.length > 0) { + let lastModel = models[models.length - 1]; - if (typeof lastModel === 'object' && lastModel !== null && lastModel.isQueryParams) { - this.query = lastModel.values; - models.pop(); + if (typeof lastModel === 'object' && lastModel !== null && lastModel.isQueryParams) { + this.query = lastModel.values; + models.pop(); + } } + } else { + assert( + 'Cannot pass a QueryParams object in @models and @query at the same time', + !(this.models.length === 0 || isQueryParams(this.models[this.models.length - 1])) + ); } return; diff --git a/packages/@ember/-internals/routing/lib/system/query_params.ts b/packages/@ember/-internals/routing/lib/system/query_params.ts index fcf0259069c..bf332c60cd4 100644 --- a/packages/@ember/-internals/routing/lib/system/query_params.ts +++ b/packages/@ember/-internals/routing/lib/system/query_params.ts @@ -1,7 +1,13 @@ +import { Option } from '@glimmer/interfaces'; + export default class QueryParams { values: null | object; isQueryParams = true; - constructor(values = null) { + constructor(values: Option = null) { this.values = values; } } + +export function isQueryParams(obj: unknown): obj is QueryParams { + return typeof obj === 'object' && obj !== null && obj['isQueryParams'] === true; +} From c7404cafac37ee15a55c8376e7e34ac094454c00 Mon Sep 17 00:00:00 2001 From: Alon Bukai Date: Fri, 4 Oct 2019 16:44:13 +0300 Subject: [PATCH 4/4] Revert "feat: Assert "Cannot pass QPs object in @models and @query at same time" This reverts commit fc7cb2d8ef082d3e691107d401a1a353f27f7b9b. --- .../glimmer/lib/components/link-to.ts | 20 ++++++------------- .../routing/lib/system/query_params.ts | 8 +------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/link-to.ts b/packages/@ember/-internals/glimmer/lib/components/link-to.ts index ef724e43a61..e57bddd465b 100644 --- a/packages/@ember/-internals/glimmer/lib/components/link-to.ts +++ b/packages/@ember/-internals/glimmer/lib/components/link-to.ts @@ -3,7 +3,6 @@ */ import { alias, computed } from '@ember/-internals/metal'; -import { isQueryParams } from '@ember/-internals/routing/lib/system/query_params'; import { isSimpleClick } from '@ember/-internals/views'; import { assert, warn } from '@ember/debug'; import { flaggedInstrument } from '@ember/instrumentation'; @@ -839,21 +838,14 @@ const LinkComponent = EmberComponent.extend({ ) ); - if (this.query === UNDEFINED) { - let { _models: models } = this; - if (models.length > 0) { - let lastModel = models[models.length - 1]; + let { _models: models } = this; + if (models.length > 0) { + let lastModel = models[models.length - 1]; - if (typeof lastModel === 'object' && lastModel !== null && lastModel.isQueryParams) { - this.query = lastModel.values; - models.pop(); - } + if (typeof lastModel === 'object' && lastModel !== null && lastModel.isQueryParams) { + this.query = lastModel.values; + models.pop(); } - } else { - assert( - 'Cannot pass a QueryParams object in @models and @query at the same time', - !(this.models.length === 0 || isQueryParams(this.models[this.models.length - 1])) - ); } return; diff --git a/packages/@ember/-internals/routing/lib/system/query_params.ts b/packages/@ember/-internals/routing/lib/system/query_params.ts index bf332c60cd4..fcf0259069c 100644 --- a/packages/@ember/-internals/routing/lib/system/query_params.ts +++ b/packages/@ember/-internals/routing/lib/system/query_params.ts @@ -1,13 +1,7 @@ -import { Option } from '@glimmer/interfaces'; - export default class QueryParams { values: null | object; isQueryParams = true; - constructor(values: Option = null) { + constructor(values = null) { this.values = values; } } - -export function isQueryParams(obj: unknown): obj is QueryParams { - return typeof obj === 'object' && obj !== null && obj['isQueryParams'] === true; -}