From 1761ea57aef85759e99bcdcc8070047124c65275 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 21 Sep 2018 20:53:50 -0400 Subject: [PATCH 01/23] first draft --- .../0000-router-service-computed-query-params | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 text/0000-router-service-computed-query-params diff --git a/text/0000-router-service-computed-query-params b/text/0000-router-service-computed-query-params new file mode 100644 index 0000000000..47f1cdb0c1 --- /dev/null +++ b/text/0000-router-service-computed-query-params @@ -0,0 +1,61 @@ +- Start Date: 2018-09-21 +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Query Params as a Computed Property on the RouterService + +## Summary + +Access to query params is currently restricted to the controller, and subsequently, the corresponding controller. +This results in some limitations with which users may consume query param data. +By exposing query params as a computed property on the `RouterService`, users will be able to more + +## Motivation + +Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. +However, like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), +it is common to have a need perform routing behavior from deep down a component tree. + +### Examples + +Having query params accessible on the router service would allow users to implement: + + - query-param aware modals that may hide or show depending on the presence of a query param. + - fill in form fields from a link. + - filter / search components could update the query param property. + +## Detailed design + +Add a computed property that splits out `window.location.search` into an object, which could have deeply nested objects and arrays. +This may need to be an observer that sets a property, +depending on what information we can derive from existing computed properties. + +## How we teach this + +The `RouterService` api documentation will need to be updated with examples on how to get and set the query params. + +This could be the primary we use query params, instead of the controller params. + +## Drawbacks + +Anyone relying on behavior of the current way query params are implemented may not be able to _exactly_ have the same behavior. +This couples the query params to the `RouterService`. +This may be a good thing, as the current way queryParams are used is somewhat awkward. + +## Alternatives + +React Router, for example, includes the query params as a string with every Route component via the `location` object. +They also provide url segments, which may also be handy, but for now, may be outside the scope of this RFC. + +The `RouterService` also has a `location` object, which could have the `search` property added to it, +like the native [location](https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils/search) has. + +If only `search` was exposed, it would allow people to continue to use the controller implementation of query params, +and then define a series of computed properties that depend on `'location.search'` to parse out their query param values. + +## Unresolved questions + +- What behavior are people using with query params that computed properties defined on the `RouterService` would not allow? +- if query params become a computed property on the `RouterService`, should they also be aliased inside the `Route`? + - this would consequently eliminate the need to setup query params in the controller + - but would beg the question of how to set defaults \ No newline at end of file From 60f5cd14c5f9b506a57e7883eb98458970034973 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 21 Sep 2018 20:55:48 -0400 Subject: [PATCH 02/23] need to have extension, oops --- ...-query-params => 0000-router-service-computed-query-params.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{0000-router-service-computed-query-params => 0000-router-service-computed-query-params.md} (100%) diff --git a/text/0000-router-service-computed-query-params b/text/0000-router-service-computed-query-params.md similarity index 100% rename from text/0000-router-service-computed-query-params rename to text/0000-router-service-computed-query-params.md From da6ac61b5f6d43aec0efb467cc4523e31b7e02d3 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 21 Sep 2018 21:01:03 -0400 Subject: [PATCH 03/23] phrasing updates --- text/0000-router-service-computed-query-params.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 47f1cdb0c1..2a86c73216 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -6,15 +6,15 @@ ## Summary -Access to query params is currently restricted to the controller, and subsequently, the corresponding controller. +Access to query params is currently restricted to the controller, and subsequently, the corresponding route. This results in some limitations with which users may consume query param data. -By exposing query params as a computed property on the `RouterService`, users will be able to more +By exposing query params as a computed property on the `RouterService`, users will be able to easily access the params from deep down a component tree, removing the need to pass the params down many levels. ## Motivation Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. However, like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), -it is common to have a need perform routing behavior from deep down a component tree. +it is common to have a need to perform routing behavior from deep down a component tree. ### Examples From 9a78296f3dee9388395e74758fb2269472a96788 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Fri, 21 Sep 2018 21:02:27 -0400 Subject: [PATCH 04/23] phrasing --- text/0000-router-service-computed-query-params.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 2a86c73216..ec09d5790a 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -13,16 +13,17 @@ By exposing query params as a computed property on the `RouterService`, users wi ## Motivation Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. -However, like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), +Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), it is common to have a need to perform routing behavior from deep down a component tree. ### Examples Having query params accessible on the router service would allow users to implement: - - query-param aware modals that may hide or show depending on the presence of a query param. + - query param aware modals that may hide or show depending on the presence of a query param. - fill in form fields from a link. - filter / search components could update the query param property. + - whatever else query params are used for outside of a SPA. ## Detailed design From 07cc589ae652430edb60bf49ef84c2bd7c8e2308 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sat, 22 Sep 2018 20:56:19 -0400 Subject: [PATCH 05/23] clarify that setting the query params computed property should update the URL --- text/0000-router-service-computed-query-params.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index ec09d5790a..63c99185bd 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -31,6 +31,8 @@ Add a computed property that splits out `window.location.search` into an object, This may need to be an observer that sets a property, depending on what information we can derive from existing computed properties. +Ensure that setting any deeply nested value in the query params object computed property also updates the URL. + ## How we teach this The `RouterService` api documentation will need to be updated with examples on how to get and set the query params. From e1956f8821545bc2a61d3171091d2b3056d0d36c Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sat, 22 Sep 2018 20:58:15 -0400 Subject: [PATCH 06/23] update questions --- text/0000-router-service-computed-query-params.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 63c99185bd..f9d9149785 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -59,6 +59,6 @@ and then define a series of computed properties that depend on `'location.search ## Unresolved questions - What behavior are people using with query params that computed properties defined on the `RouterService` would not allow? -- if query params become a computed property on the `RouterService`, should they also be aliased inside the `Route`? - - this would consequently eliminate the need to setup query params in the controller - - but would beg the question of how to set defaults \ No newline at end of file +- if query params become a computed property on the `RouterService`, should they also be aliased inside the `Controller` and / or `Route`? + - this would consequently eliminate the need to setup query params using the existing method + - how do we set defaults? From e3efde6c567f7b1c805e0bed3b9612ea755669b3 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 28 Sep 2018 19:54:24 -0400 Subject: [PATCH 07/23] Update 0000-router-service-computed-query-params.md --- text/0000-router-service-computed-query-params.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index f9d9149785..96b4b3a70c 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -37,7 +37,7 @@ Ensure that setting any deeply nested value in the query params object computed The `RouterService` api documentation will need to be updated with examples on how to get and set the query params. -This could be the primary we use query params, instead of the controller params. +This could be the primary way we use query params, instead of the controller params. ## Drawbacks From cac8af8cee2270b8fc273488abbd3c9265529e34 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Sat, 29 Sep 2018 06:16:25 -0400 Subject: [PATCH 08/23] Update 0000-router-service-computed-query-params.md - Make the design a little clearer - specify that the query params must have a dependent key on the route name - add code to "How we teach this" for before and after the RFC --- ...00-router-service-computed-query-params.md | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 96b4b3a70c..4e4a012c53 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -28,16 +28,49 @@ Having query params accessible on the router service would allow users to implem ## Detailed design Add a computed property that splits out `window.location.search` into an object, which could have deeply nested objects and arrays. -This may need to be an observer that sets a property, -depending on what information we can derive from existing computed properties. +The computed property must contain a dependent key on the route path, as the same queury parame may be used differently on different routes. +The computed property must return an object containing computed properties for allowing behavior bound to the value of a query param. Ensure that setting any deeply nested value in the query params object computed property also updates the URL. ## How we teach this -The `RouterService` api documentation will need to be updated with examples on how to get and set the query params. - -This could be the primary way we use query params, instead of the controller params. +Currently, query params _must_ be [specified on the controller](https://guides.emberjs.com/release/routing/query-params/): +```ts +export default class extends Controller { + queryParams = ['page', 'filter', { + // QP 'articles_category' is mapped to 'category' in our route and controller + category: 'articles_category' + }; + category = null; + page = 1; + filter = 'recent'; + + @computed('category', 'model') + get filteredArticles() { + // do something with category and model as category changes + } +} +``` + +Having computed properties available from the `RouterService` would look like this: + +```ts +export default class extends Controller { + @service router; + + @readOnly('router.queryParams') query; + + @alias('query.articles_category') category; + @alias('query.page') page = 1; + @alias('query.filter') filter = 'recent'; + + @computed('category', 'model') + get filteredArticles() { + // do something with category and model as category changes + } +} +``` ## Drawbacks @@ -59,6 +92,3 @@ and then define a series of computed properties that depend on `'location.search ## Unresolved questions - What behavior are people using with query params that computed properties defined on the `RouterService` would not allow? -- if query params become a computed property on the `RouterService`, should they also be aliased inside the `Controller` and / or `Route`? - - this would consequently eliminate the need to setup query params using the existing method - - how do we set defaults? From 638d2cf57d17aa2a250a24e10fb25872d064a710 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Sat, 29 Sep 2018 06:17:18 -0400 Subject: [PATCH 09/23] Update 0000-router-service-computed-query-params.md --- text/0000-router-service-computed-query-params.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 4e4a012c53..c3e40f0167 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -41,7 +41,7 @@ export default class extends Controller { queryParams = ['page', 'filter', { // QP 'articles_category' is mapped to 'category' in our route and controller category: 'articles_category' - }; + }]; category = null; page = 1; filter = 'recent'; From 1d8ddddbec386203cbee29dddac172885158b8d4 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 6 Jun 2019 12:46:11 -0400 Subject: [PATCH 10/23] light updates --- ...00-router-service-computed-query-params.md | 238 +++++++++++++++--- 1 file changed, 206 insertions(+), 32 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index c3e40f0167..4f99edd5a4 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -8,16 +8,18 @@ Access to query params is currently restricted to the controller, and subsequently, the corresponding route. This results in some limitations with which users may consume query param data. -By exposing query params as a computed property on the `RouterService`, users will be able to easily access the params from deep down a component tree, removing the need to pass the params down many levels. +By exposing query params on the `RouterService`, or a separate `QueryParamsService`, users will be able to easily access the params from deep down a component tree, removing the need to pass query-param related data and actions down many layers of components. ## Motivation Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), -it is common to have a need to perform routing behavior from deep down a component tree. +it is common to have a need to perform routing behavior from deep down a component tree. ### Examples +An addon has been written to demonstrate usage: https://github.com/NullVoxPopuli/ember-query-params-service + Having query params accessible on the router service would allow users to implement: - query param aware modals that may hide or show depending on the presence of a query param. @@ -27,11 +29,190 @@ Having query params accessible on the router service would allow users to implem ## Detailed design -Add a computed property that splits out `window.location.search` into an object, which could have deeply nested objects and arrays. -The computed property must contain a dependent key on the route path, as the same queury parame may be used differently on different routes. -The computed property must return an object containing computed properties for allowing behavior bound to the value of a query param. - +Add a service that maintains properties derived from a split of `window.location.search` into objects, which could have deeply nested objects and arrays. +The properties must maintain state based on the current route, which mimics the current behavior that the singleton controller's long-lived state provide with respect to maintaining query-param values. Ensure that setting any deeply nested value in the query params object computed property also updates the URL. +A `queryParam` decorator for accessing and updating query params in the URL must exist for short-hand use in components, routes, etc. + +The biggest change needed, which could potentially be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed as people transition to this form of queryParams management. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. +This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. + +Example implementation of the service: +```ts +import Service, { inject as service } from '@ember/service'; +import RouterService from '@ember/routing/router-service'; + +import { tracked } from '@glimmer/tracking'; +import * as qs from 'qs'; + +interface QueryParams { + [key: string]: number | string | undefined | QueryParams; +} + +interface QueryParamsByPath { + [key: string]: QueryParams; +} + +export default class QueryParamsService extends Service { + @service router!: RouterService; + + @tracked current!: QueryParams; + @tracked byPath: QueryParamsByPath = {}; + + constructor(...args: any[]) { + super(...args); + + this.setupProxies(); + } + + init() { + super.init(); + + this.updateParams(); + + this.router.on('routeDidChange', () => this.updateParams()); + this.router.on('routeWillChange', () => this.updateParams()); + } + + get pathParts() { + const [path, params] = (this.router.currentURL || '').split('?'); + + return [path, params]; + } + + private setupProxies() { + let [path] = this.pathParts; + + this.byPath[path] = this.byPath[path] || {}; + + this.current = new Proxy(this.byPath[path], queryParamHandler); + } + + private updateParams() { + this.setupProxies(); + + const [path, params] = this.pathParts; + const queryParams = params && qs.parse(params); + + Object.keys(queryParams || {}).forEach(key => { + let value = queryParams[key]; + let currentValue = this.byPath[path][key]; + + if (currentValue === value) { + return; + } + + this.byPath[path][key] = value; + }); + } +} + +const queryParamHandler = { + get(obj: any, key: string, ...rest: any[]) { + return Reflect.get(obj, key, ...rest); + }, + set(obj: any, key: string, value: any, ...rest: any[]) { + let { protocol, host, pathname } = window.location; + let query = qs.stringify({ ...obj, [key]: value }); + let newUrl = `${protocol}//${host}${pathname}?${query}`; + + window.history.pushState({ path: newUrl }, '', newUrl); + + return Reflect.set(obj, key, value, ...rest); + }, +}; +``` + +Example implementation of the decorator: + +```ts +import { get, set } from '@ember/object'; +import { getOwner } from '@ember/application'; +import { tracked } from '@glimmer/tracking'; +import { default as QueryParamsService } from '../services/query-params'; + +export interface ITransformOptions { + deserialize?: (queryParam: string) => T; + serialize?: (queryParam: T) => string; +} + +type Args = [] | [string, ITransformOptions] | [ITransformOptions] | [string]; + +export function queryParam(...args: Args) { + return (target: any, propertyKey: string, sourceDescriptor?: any) => { + const { set: oldSet } = tracked(target, propertyKey, sourceDescriptor); + const [propertyPath, options] = extractArgs(args, propertyKey); + + const result = { + configurable: true, + enumerable: true, + get: function(): T { + // setupController(this, 'application'); + const service = ensureService(this); + const value = get(service, propertyPath); + const deserialized = tryDeserialize(value, options); + + return deserialized; + }, + set: function(value: any) { + // setupController(this, 'application'); + const service = ensureService(this); + const serialized = trySerialize(value, options); + + set(service, propertyPath, serialized); + oldSet!.call(this, serialized); + }, + }; + + return result as any; + }; +} + +function extractArgs(args: Args, propertyKey: string): [string, ITransformOptions] { + const [maybePathMaybeOptions, maybeOptions] = args; + + let propertyPath: string; + let options: ITransformOptions; + + if (typeof maybePathMaybeOptions === 'string') { + propertyPath = `current.${maybePathMaybeOptions}`; + options = maybeOptions || {}; + } else { + propertyPath = `current.${propertyKey}`; + options = maybePathMaybeOptions || {}; + } + + return [propertyPath, options]; +} + +function tryDeserialize(value: any, options: ITransformOptions) { + if (!options.deserialize) return value; + + return options.deserialize(value); +} + +function trySerialize(value: any, options: ITransformOptions) { + if (!options.serialize) return value; + + return options.serialize(value); +} + +// could there ever be a problem with using only one variable in module-space? +let qpService: QueryParamsService; +function ensureService(context: any): QueryParamsService { + if (qpService) { + return qpService; + } + + qpService = getQPService(context); + + return qpService; +} + +function getQPService(context: any) { + return getOwner(context).lookup('service:queryParams'); +} +``` ## How we teach this @@ -53,42 +234,35 @@ export default class extends Controller { } ``` -Having computed properties available from the `RouterService` would look like this: +Having computed properties available elsewhere will be a shift in thinking that "the controller manages query params" to "the service that allows access to the query params manages the query params" ```ts -export default class extends Controller { - @service router; - - @readOnly('router.queryParams') query; - - @alias('query.articles_category') category; - @alias('query.page') page = 1; - @alias('query.filter') filter = 'recent'; - - @computed('category', 'model') - get filteredArticles() { - // do something with category and model as category changes +import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; +import { alias } from 'ember-query-params-service'; + +export default class ApplicationRoute extends Route { + @service queryParams; + + @alias('queryParams.current.r') isSpeakerNotes; + @alias('queryParams.current.slide') slideNumber; + + model() { + return { + isSpeakerNotes: this.isSpeakerNotes, + slideNumber: this.slideNumber + } } } ``` ## Drawbacks -Anyone relying on behavior of the current way query params are implemented may not be able to _exactly_ have the same behavior. -This couples the query params to the `RouterService`. -This may be a good thing, as the current way queryParams are used is somewhat awkward. +- This requires tracked properties as the tracking system is perfect for propagating updates whenever the internal queryParam tracking object changes -- in the QueryParamsService's `byPath` and `current` properties. So only the most up-to-date code-bases would be able to benefit. +- The old behavior of controller-based query params where query params live on the controller that is a singleton and are restored whenever a route is returned to is still possible / implemented with this QueryParamService, but because components can modify query params at any depth, this may introduce hard-to-trace behaviors. +- Some people may be relying on the controller query-params allow-list. ## Alternatives React Router, for example, includes the query params as a string with every Route component via the `location` object. They also provide url segments, which may also be handy, but for now, may be outside the scope of this RFC. - -The `RouterService` also has a `location` object, which could have the `search` property added to it, -like the native [location](https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils/search) has. - -If only `search` was exposed, it would allow people to continue to use the controller implementation of query params, -and then define a series of computed properties that depend on `'location.search'` to parse out their query param values. - -## Unresolved questions - -- What behavior are people using with query params that computed properties defined on the `RouterService` would not allow? From f62967b32e8ccb6c3c0ac74eb4899621491afa81 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 6 Jun 2019 12:47:03 -0400 Subject: [PATCH 11/23] Update 0000-router-service-computed-query-params.md --- text/0000-router-service-computed-query-params.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 4f99edd5a4..7bd7f830af 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -2,13 +2,13 @@ - RFC PR: (leave this empty) - Ember Issue: (leave this empty) -# Query Params as a Computed Property on the RouterService +# Add `@queryParam` decorator and `QueryParamsService` ## Summary Access to query params is currently restricted to the controller, and subsequently, the corresponding route. This results in some limitations with which users may consume query param data. -By exposing query params on the `RouterService`, or a separate `QueryParamsService`, users will be able to easily access the params from deep down a component tree, removing the need to pass query-param related data and actions down many layers of components. +By exposing query params on a new `QueryParamsService`, users will be able to easily access the params from deep down a component tree, removing the need to pass query-param related data and actions down many layers of components. ## Motivation From c688e7018f2ef3b55d997dbbd560f4d647c127d1 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 6 Jun 2019 12:57:40 -0400 Subject: [PATCH 12/23] Update 0000-router-service-computed-query-params.md --- text/0000-router-service-computed-query-params.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 7bd7f830af..8c3c929d21 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -1,5 +1,5 @@ - Start Date: 2018-09-21 -- RFC PR: (leave this empty) +- RFC PR: [https://github.com/emberjs/rfcs/pull/380](https://github.com/emberjs/rfcs/pull/380) - Ember Issue: (leave this empty) # Add `@queryParam` decorator and `QueryParamsService` @@ -15,6 +15,7 @@ By exposing query params on a new `QueryParamsService`, users will be able to ea Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), it is common to have a need to perform routing behavior from deep down a component tree. +Additionally, the current query params implementation feels very verbose for "just wanting to access a property". Accessing data within the url **should feel easy**. ### Examples From 52e454c1ba6d3f5410cf159039b2ca4bd06ab20d Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Thu, 6 Jun 2019 13:01:06 -0400 Subject: [PATCH 13/23] Update 0000-router-service-computed-query-params.md --- text/0000-router-service-computed-query-params.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 8c3c929d21..8a16925c95 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -44,6 +44,9 @@ import Service, { inject as service } from '@ember/service'; import RouterService from '@ember/routing/router-service'; import { tracked } from '@glimmer/tracking'; +// this does use an external dependency +// it's lightweight though, 3.4kb, according to bundlephobia.com +// we implement this ourselves, or maybe it already exists somewhere in ember import * as qs from 'qs'; interface QueryParams { @@ -57,7 +60,12 @@ interface QueryParamsByPath { export default class QueryParamsService extends Service { @service router!: RouterService; + // the current set of query params for whatever the current route is @tracked current!: QueryParams; + + // stores the "current" query params object for each route based on path. + // this allows for restoring query params values when switching + // between routes @tracked byPath: QueryParamsByPath = {}; constructor(...args: any[]) { From 1d72f0714da5614113cbc4674ed85ac9d56b56f9 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 7 Jun 2019 11:45:02 -0400 Subject: [PATCH 14/23] Add note about updating the model hook when tracked properties change --- text/0000-router-service-computed-query-params.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 8a16925c95..db30767fcd 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -38,6 +38,8 @@ A `queryParam` decorator for accessing and updating query params in the URL must The biggest change needed, which could potentially be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed as people transition to this form of queryParams management. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. +Additionally, in order to be backwards-compatible with `{ refreshModel: true }`, the model hook will need to re-run if any tracked properties used within the model hook are changed. + Example implementation of the service: ```ts import Service, { inject as service } from '@ember/service'; From 3438cc194b3ae385029735315f0f18f51e5630fc Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 7 Jun 2019 13:48:10 -0400 Subject: [PATCH 15/23] Update 0000-router-service-computed-query-params.md --- ...00-router-service-computed-query-params.md | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index db30767fcd..ae2adc716e 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -46,9 +46,6 @@ import Service, { inject as service } from '@ember/service'; import RouterService from '@ember/routing/router-service'; import { tracked } from '@glimmer/tracking'; -// this does use an external dependency -// it's lightweight though, 3.4kb, according to bundlephobia.com -// we implement this ourselves, or maybe it already exists somewhere in ember import * as qs from 'qs'; interface QueryParams { @@ -62,12 +59,7 @@ interface QueryParamsByPath { export default class QueryParamsService extends Service { @service router!: RouterService; - // the current set of query params for whatever the current route is @tracked current!: QueryParams; - - // stores the "current" query params object for each route based on path. - // this allows for restoring query params values when switching - // between routes @tracked byPath: QueryParamsByPath = {}; constructor(...args: any[]) { @@ -82,13 +74,14 @@ export default class QueryParamsService extends Service { this.updateParams(); this.router.on('routeDidChange', () => this.updateParams()); - this.router.on('routeWillChange', () => this.updateParams()); + this.router.on('routeWillChange', transition => this.updateURL(transition)); } get pathParts() { const [path, params] = (this.router.currentURL || '').split('?'); + const absolutePath = path.startsWith('/') ? path : `/${path}`; - return [path, params]; + return [absolutePath, params]; } private setupProxies() { @@ -105,6 +98,28 @@ export default class QueryParamsService extends Service { const [path, params] = this.pathParts; const queryParams = params && qs.parse(params); + this.setOnPath(path, queryParams); + } + + /** + * When we have stored query params for a route + * throw them on the URL + * + */ + private updateURL(transition: any /* Transition doesn't have intent.. some how? */) { + const path = transition.intent.url; + const { protocol, host, pathname, search } = window.location; + const queryParams = this.byPath[path]; + const existing = qs.parse(search.split('?')[1]); + const query = qs.stringify({ ...existing, ...queryParams }); + const newUrl = `${protocol}//${host}${pathname}?${query}`; + + window.history.replaceState({ path: newUrl }, '', newUrl); + } + + private setOnPath(path: string, queryParams: object) { + this.byPath[path] = this.byPath[path] || {}; + Object.keys(queryParams || {}).forEach(key => { let value = queryParams[key]; let currentValue = this.byPath[path][key]; @@ -113,6 +128,11 @@ export default class QueryParamsService extends Service { return; } + if (value === undefined) { + delete this.byPath[path][key]; + return; + } + this.byPath[path][key] = value; }); } @@ -132,6 +152,7 @@ const queryParamHandler = { return Reflect.set(obj, key, value, ...rest); }, }; + ``` Example implementation of the decorator: From acf9ee9dc67d9b4243dfcdf4974bd6a465b8f475 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Tue, 11 Jun 2019 07:02:04 -0400 Subject: [PATCH 16/23] Update 0000-router-service-computed-query-params.md --- ...00-router-service-computed-query-params.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index ae2adc716e..2384948e09 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -21,6 +21,20 @@ Additionally, the current query params implementation feels very verbose for "ju An addon has been written to demonstrate usage: https://github.com/NullVoxPopuli/ember-query-params-service +Example usage: +```ts +import Component from "@glimmer/component"; +import { queryParam } from "ember-query-params-service"; + +export default class SomeComponent extends Component { + @queryParam foo; + + addToFoo() { + this.foo = (this.foo || 0) + 1; + } +} +``` + Having query params accessible on the router service would allow users to implement: - query param aware modals that may hide or show depending on the presence of a query param. @@ -265,6 +279,21 @@ export default class extends Controller { } } ``` +The default happy path should be one that suggests usage of the `@queryParam` decorator. This is the most flexible, and prevides serialization/deserialization options per-query param if needed. Maybe using [@pzuraq's macro-decorators](https://pzuraq.github.io/macro-decorators/), users could wrap `@queryParam` for different data types. + +```ts +import Component from "@glimmer/component"; +import { queryParam } from "ember-query-params-service"; + +export default class SomeComponent extends Component { + @queryParam foo; + + addToFoo() { + this.foo = (this.foo || 0) + 1; + } +} +``` + Having computed properties available elsewhere will be a shift in thinking that "the controller manages query params" to "the service that allows access to the query params manages the query params" From aceca2501f74654342887479a4e8a7685d04ff7e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Tue, 11 Jun 2019 17:46:10 -0400 Subject: [PATCH 17/23] Lots of updates / redisign --- ...00-router-service-computed-query-params.md | 318 ++++++------------ 1 file changed, 104 insertions(+), 214 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 2384948e09..b283b91304 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -2,35 +2,42 @@ - RFC PR: [https://github.com/emberjs/rfcs/pull/380](https://github.com/emberjs/rfcs/pull/380) - Ember Issue: (leave this empty) -# Add `@queryParam` decorator and `QueryParamsService` +# Add `queryParams` to the router service ## Summary +This RFC proposes a new primitive API change to the `RouterService` to allow access to query params from anywhere. + Access to query params is currently restricted to the controller, and subsequently, the corresponding route. This results in some limitations with which users may consume query param data. -By exposing query params on a new `QueryParamsService`, users will be able to easily access the params from deep down a component tree, removing the need to pass query-param related data and actions down many layers of components. +By exposing query params on the [RouterService](https://api.emberjs.com/ember/release/classes/RouterService), users will be able to easily access the query params from deep down a component tree, removing the need to pass query param related data and actions down many layers of components. + + ## Motivation -Modern SPA concepts have converged on the idea that query params should be easily accessible from the object responsible for handling the route. +Modern SPA concepts have converged on the idea that query params should be easily accessible -- independent from the object responsible for handling the route. Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), it is common to have a need to perform routing behavior from deep down a component tree. -Additionally, the current query params implementation feels very verbose for "just wanting to access a property". Accessing data within the url **should feel easy**. +Additionally, the current query params implementation feels very verbose for "just wanting to access a property" and has been frustrating to have to explain awkward behavior when on-boarding new devs who may be unfamiliar with Ember. -### Examples +Accessing data within the url **should feel easy**. -An addon has been written to demonstrate usage: https://github.com/NullVoxPopuli/ember-query-params-service +## Detailed Design + +### Accessing Query Params -Example usage: ```ts import Component from "@glimmer/component"; -import { queryParam } from "ember-query-params-service"; +import { inject as service } from '@ember/service'; -export default class SomeComponent extends Component { - @queryParam foo; +export default class Pagination extends Component { + @service router; - addToFoo() { - this.foo = (this.foo || 0) + 1; + get currentPage() { + const { page } = this.router.queryParams; + + return page; } } ``` @@ -42,223 +49,121 @@ Having query params accessible on the router service would allow users to implem - filter / search components could update the query param property. - whatever else query params are used for outside of a SPA. -## Detailed design - -Add a service that maintains properties derived from a split of `window.location.search` into objects, which could have deeply nested objects and arrays. -The properties must maintain state based on the current route, which mimics the current behavior that the singleton controller's long-lived state provide with respect to maintaining query-param values. -Ensure that setting any deeply nested value in the query params object computed property also updates the URL. -A `queryParam` decorator for accessing and updating query params in the URL must exist for short-hand use in components, routes, etc. +### Transitioning to Add and Remove Query Params -The biggest change needed, which could potentially be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed as people transition to this form of queryParams management. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. -This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. - -Additionally, in order to be backwards-compatible with `{ refreshModel: true }`, the model hook will need to re-run if any tracked properties used within the model hook are changed. - -Example implementation of the service: -```ts -import Service, { inject as service } from '@ember/service'; -import RouterService from '@ember/routing/router-service'; - -import { tracked } from '@glimmer/tracking'; -import * as qs from 'qs'; - -interface QueryParams { - [key: string]: number | string | undefined | QueryParams; -} - -interface QueryParamsByPath { - [key: string]: QueryParams; -} +Given there is a route that exists named `avengers.roster`, -export default class QueryParamsService extends Service { - @service router!: RouterService; + - Adding a query param + ```ts + import Component from "@glimmer/component"; + import { inject as service } from '@ember/service'; + import { action } from '@ember/object'; - @tracked current!: QueryParams; - @tracked byPath: QueryParamsByPath = {}; + export default class Pagination extends Component { + @service router; - constructor(...args: any[]) { - super(...args); + @action nextPage() { + // `this.router.queryParams` is currently an empty object + const nextPage = (this.router.queryParams.page || 0) + 1 - this.setupProxies(); - } - - init() { - super.init(); - - this.updateParams(); - - this.router.on('routeDidChange', () => this.updateParams()); - this.router.on('routeWillChange', transition => this.updateURL(transition)); - } - - get pathParts() { - const [path, params] = (this.router.currentURL || '').split('?'); - const absolutePath = path.startsWith('/') ? path : `/${path}`; - - return [absolutePath, params]; - } - - private setupProxies() { - let [path] = this.pathParts; + this.router.transitionTo('avengers.roster', { queryParams: { page: nextPage } }); + // after the transition, `this.router.queryParams` is `{ page: 1 }` + } + } + ``` - this.byPath[path] = this.byPath[path] || {}; + - Removing all query params + ```ts + import Component from "@glimmer/component"; + import { inject as service } from '@ember/service'; + import { action } from '@ember/object'; - this.current = new Proxy(this.byPath[path], queryParamHandler); - } + export default class Pagination extends Component { + @service router; - private updateParams() { - this.setupProxies(); + @action reset() { + // `this.router.queryParams` is currently `{ page: 1 }` - const [path, params] = this.pathParts; - const queryParams = params && qs.parse(params); + this.router.transitionTo('avengers.roster', { queryParams: { /* empty */ } }); + // after the transition, `this.router.queryParams` is `{ }` + } + } + ``` - this.setOnPath(path, queryParams); - } + - Merging query params - /** - * When we have stored query params for a route - * throw them on the URL - * - */ - private updateURL(transition: any /* Transition doesn't have intent.. some how? */) { - const path = transition.intent.url; - const { protocol, host, pathname, search } = window.location; - const queryParams = this.byPath[path]; - const existing = qs.parse(search.split('?')[1]); - const query = qs.stringify({ ...existing, ...queryParams }); - const newUrl = `${protocol}//${host}${pathname}?${query}`; - - window.history.replaceState({ path: newUrl }, '', newUrl); - } + We have previously been to the route `avengers.roster` with query params `{ page: 1 }` + ```ts + import Component from "@glimmer/component"; + import { inject as service } from '@ember/service'; + import { action } from '@ember/object'; - private setOnPath(path: string, queryParams: object) { - this.byPath[path] = this.byPath[path] || {}; + export default class HeaderNavigation extends Component { + @service router; - Object.keys(queryParams || {}).forEach(key => { - let value = queryParams[key]; - let currentValue = this.byPath[path][key]; + @action navigateToRoster() { + // `this.router.currentRouteName` is currently `'index'` + // `this.router.queryParams` is currently `{ }` - if (currentValue === value) { - return; - } - - if (value === undefined) { - delete this.byPath[path][key]; - return; + this.router.transitionTo(`avengers.roster`); + // after the transition, `this.router.queryParams` has restored the value of `{ page: 1 }` + // from the controller state } + } + ``` - this.byPath[path][key] = value; - }); - } -} -const queryParamHandler = { - get(obj: any, key: string, ...rest: any[]) { - return Reflect.get(obj, key, ...rest); - }, - set(obj: any, key: string, value: any, ...rest: any[]) { - let { protocol, host, pathname } = window.location; - let query = qs.stringify({ ...obj, [key]: value }); - let newUrl = `${protocol}//${host}${pathname}?${query}`; +The biggest change needed, which could _potentially_ be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. +This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. - window.history.pushState({ path: newUrl }, '', newUrl); - return Reflect.set(obj, key, value, ...rest); - }, -}; +### Setting Query Params -``` +Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon: https://github.com/NullVoxPopuli/ember-query-params-service. -Example implementation of the decorator: +This is powered by the [Proxy object, here](https://github.com/NullVoxPopuli/ember-query-params-service/blob/master/addon/services/query-params.ts#L49). ```ts -import { get, set } from '@ember/object'; -import { getOwner } from '@ember/application'; -import { tracked } from '@glimmer/tracking'; -import { default as QueryParamsService } from '../services/query-params'; - -export interface ITransformOptions { - deserialize?: (queryParam: string) => T; - serialize?: (queryParam: T) => string; -} - -type Args = [] | [string, ITransformOptions] | [ITransformOptions] | [string]; - -export function queryParam(...args: Args) { - return (target: any, propertyKey: string, sourceDescriptor?: any) => { - const { set: oldSet } = tracked(target, propertyKey, sourceDescriptor); - const [propertyPath, options] = extractArgs(args, propertyKey); - - const result = { - configurable: true, - enumerable: true, - get: function(): T { - // setupController(this, 'application'); - const service = ensureService(this); - const value = get(service, propertyPath); - const deserialized = tryDeserialize(value, options); - - return deserialized; - }, - set: function(value: any) { - // setupController(this, 'application'); - const service = ensureService(this); - const serialized = trySerialize(value, options); - - set(service, propertyPath, serialized); - oldSet!.call(this, serialized); - }, - }; - - return result as any; - }; -} - -function extractArgs(args: Args, propertyKey: string): [string, ITransformOptions] { - const [maybePathMaybeOptions, maybeOptions] = args; +import Component from "@glimmer/component"; +import { queryParam } from "ember-query-params-service"; - let propertyPath: string; - let options: ITransformOptions; +export default class SomeComponent extends Component { + @queryParam strongestAvenger = 'Hulk'; - if (typeof maybePathMaybeOptions === 'string') { - propertyPath = `current.${maybePathMaybeOptions}`; - options = maybeOptions || {}; - } else { - propertyPath = `current.${propertyKey}`; - options = maybePathMaybeOptions || {}; + updateStrongestAvenger() { + this.strongestAvenger = 'Thor'; } - - return [propertyPath, options]; } +``` -function tryDeserialize(value: any, options: ITransformOptions) { - if (!options.deserialize) return value; +This is due to the fact that IE11 only supports ES5, which [does not have Proxy support](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy#Browser_compatibility) and [cannot be polyfilled](https://babeljs.io/docs/en/learn#proxies). - return options.deserialize(value); -} +> Due to the limitations of ES5, Proxies cannot be transpiled or polyfilled. -function trySerialize(value: any, options: ITransformOptions) { - if (!options.serialize) return value; +- _[https://babeljs.io/docs/en/learn#proxies](https://babeljs.io/docs/en/learn#proxies) - return options.serialize(value); -} -// could there ever be a problem with using only one variable in module-space? -let qpService: QueryParamsService; -function ensureService(context: any): QueryParamsService { - if (qpService) { - return qpService; - } +Instead, functions must be defined on the router that would take care of the setting of the query param's value. +```ts +// set a single parameter +this.router.setQueryParameter('strongestAvenger', 'Captain Marvel'); +// set many parameters +// this'll replace +this.router.setQueryParameters({ + strongestAvenger: 'Carol Danvers', + secondStrongest: 'Thor or Hulk?', + ['kebab-case-param']: `kebab o' Thanos' armies`, +}); +``` - qpService = getQPService(context); +Just as it is today, setting query params in this way would not trigger a transition. +The key-value state of set query params would be available on `#queryParams` as well as an existing controller's computed properties as they exist today. - return qpService; -} +This doesn't change the existing functionality where objects and arrays are still not correctly serialized to their URI-string counterparts. + +To address that, a long standing issue from as far back as 2016, +some new functionality for serialization and deserialization would be powered by [qs](https://www.npmjs.com/package/qs) ([3.4kb (gzip+min)](https://bundlephobia.com/result?p=qs@6.7.0)) -- which would enable the setting of arrays and objects. -function getQPService(context: any) { - return getOwner(context).lookup('service:queryParams'); -} -``` ## How we teach this @@ -279,23 +184,10 @@ export default class extends Controller { } } ``` -The default happy path should be one that suggests usage of the `@queryParam` decorator. This is the most flexible, and prevides serialization/deserialization options per-query param if needed. Maybe using [@pzuraq's macro-decorators](https://pzuraq.github.io/macro-decorators/), users could wrap `@queryParam` for different data types. - -```ts -import Component from "@glimmer/component"; -import { queryParam } from "ember-query-params-service"; - -export default class SomeComponent extends Component { - @queryParam foo; - - addToFoo() { - this.foo = (this.foo || 0) + 1; - } -} -``` +This will no longer be true. However, controllers will still manage the long-term state of the query params as they do today. -Having computed properties available elsewhere will be a shift in thinking that "the controller manages query params" to "the service that allows access to the query params manages the query params" +Having query-param-related computed properties available everywhere will be a shift in thinking that "the controller manages query params" to "query params are a routing concern and are on the router service" ```ts import Route from '@ember/routing/route'; @@ -303,10 +195,10 @@ import { inject as service } from '@ember/service'; import { alias } from 'ember-query-params-service'; export default class ApplicationRoute extends Route { - @service queryParams; + @service router; - @alias('queryParams.current.r') isSpeakerNotes; - @alias('queryParams.current.slide') slideNumber; + @alias('router.queryParams.r') isSpeakerNotes; + @alias('router.queryParams.slide') slideNumber; model() { return { @@ -319,8 +211,6 @@ export default class ApplicationRoute extends Route { ## Drawbacks -- This requires tracked properties as the tracking system is perfect for propagating updates whenever the internal queryParam tracking object changes -- in the QueryParamsService's `byPath` and `current` properties. So only the most up-to-date code-bases would be able to benefit. -- The old behavior of controller-based query params where query params live on the controller that is a singleton and are restored whenever a route is returned to is still possible / implemented with this QueryParamService, but because components can modify query params at any depth, this may introduce hard-to-trace behaviors. - Some people may be relying on the controller query-params allow-list. ## Alternatives From a1aec6c2b404a760bcac3040c36bf79b65989662 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Tue, 11 Jun 2019 17:51:12 -0400 Subject: [PATCH 18/23] Be more concise --- text/0000-router-service-computed-query-params.md | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index b283b91304..724b0ce88c 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -119,21 +119,12 @@ This is a dynamic way to manage query params which hopefully aligns more with pe ### Setting Query Params -Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon: https://github.com/NullVoxPopuli/ember-query-params-service. +Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon. -This is powered by the [Proxy object, here](https://github.com/NullVoxPopuli/ember-query-params-service/blob/master/addon/services/query-params.ts#L49). +For example, this is not possible until IE11 support is dropped: ```ts -import Component from "@glimmer/component"; -import { queryParam } from "ember-query-params-service"; - -export default class SomeComponent extends Component { - @queryParam strongestAvenger = 'Hulk'; - - updateStrongestAvenger() { - this.strongestAvenger = 'Thor'; - } -} +this.router.queryParams.strongestAvenger = 'Hulk'; ``` This is due to the fact that IE11 only supports ES5, which [does not have Proxy support](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy#Browser_compatibility) and [cannot be polyfilled](https://babeljs.io/docs/en/learn#proxies). From 7c50703e884402a55e6dcaa85d0cdd8898ba96dc Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Wed, 12 Jun 2019 09:54:14 -0400 Subject: [PATCH 19/23] Mention URLSearchParams --- text/0000-router-service-computed-query-params.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 724b0ce88c..4b7f910011 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -156,6 +156,8 @@ To address that, a long standing issue from as far back as 2016, some new functionality for serialization and deserialization would be powered by [qs](https://www.npmjs.com/package/qs) ([3.4kb (gzip+min)](https://bundlephobia.com/result?p=qs@6.7.0)) -- which would enable the setting of arrays and objects. +Alternatively, [URLSearchParams](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams) is a built-in feature of browsers, and there exists a polyfill for IE11. This might be lighter weight than pulling in qs. + ## How we teach this Currently, query params _must_ be [specified on the controller](https://guides.emberjs.com/release/routing/query-params/): From a7c2e7b7e32bc681cfd79eb61382276a3777b2c1 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 14 Jun 2019 10:48:17 -0400 Subject: [PATCH 20/23] Update 0000-router-service-computed-query-params.md --- ...00-router-service-computed-query-params.md | 118 ++++++++---------- 1 file changed, 54 insertions(+), 64 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 4b7f910011..665a38d081 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -49,74 +49,78 @@ Having query params accessible on the router service would allow users to implem - filter / search components could update the query param property. - whatever else query params are used for outside of a SPA. -### Transitioning to Add and Remove Query Params +### Serialization / Deserialization -Given there is a route that exists named `avengers.roster`, +Everyone has different query param serialization and deserialization needs depending on a variety of factors. - - Adding a query param - ```ts - import Component from "@glimmer/component"; - import { inject as service } from '@ember/service'; - import { action } from '@ember/object'; +By default, the query params will be serialized and deserialized via the builtin URLSearchParams API, and polyfilled for IE11. - export default class Pagination extends Component { - @service router; +Should someone decide to customize how serilaization and deserializion transforms the query params, that can be done directly on the router: - @action nextPage() { - // `this.router.queryParams` is currently an empty object - const nextPage = (this.router.queryParams.page || 0) + 1 - - this.router.transitionTo('avengers.roster', { queryParams: { page: nextPage } }); - // after the transition, `this.router.queryParams` is `{ page: 1 }` - } +```ts +import EmberRouter from '@ember/routing/router'; + +import config from '../config/environment'; + +const Router = EmberRouter.extend({ + location: config.locationType, + rootURL: config.rootURL, + + queryParamsConfig: { + serialize(queryParams: object): string { + // serialize object for query string + // default to URLSearchParams, polyfilled for IE11 + }, + deserialize(queryString: string): object { + // parse to object for use in `injectedRouter.queryString` + // also default to URLSerachParams } - ``` + } +}); +``` - - Removing all query params - ```ts - import Component from "@glimmer/component"; - import { inject as service } from '@ember/service'; - import { action } from '@ember/object'; - export default class Pagination extends Component { - @service router; +This will address a long standing issues from as far back as 2016, +some new functionality for serialization and deserialization could be powered by [qs](https://www.npmjs.com/package/qs) ([3.4kb (gzip+min)](https://bundlephobia.com/result?p=qs@6.7.0)) or a lternatively, [URLSearchParams](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams) -- this would enable the setting of arrays and objects which is not possible today. - @action reset() { - // `this.router.queryParams` is currently `{ page: 1 }` - this.router.transitionTo('avengers.roster', { queryParams: { /* empty */ } }); - // after the transition, `this.router.queryParams` is `{ }` - } - } - ``` +### Sticky Query Params - - Merging query params +By default, transitionTo will clear the query params, unless specified inside the transiion. - We have previously been to the route `avengers.roster` with query params `{ page: 1 }` - ```ts - import Component from "@glimmer/component"; - import { inject as service } from '@ember/service'; - import { action } from '@ember/object'; +If query params are defined ahead of time as sticky, they will persist in the URL between sub routes. - export default class HeaderNavigation extends Component { - @service router; +This can be configured in the `Router.map` function: +```ts +Router.map(function() { + this.queryParams('bar'); - @action navigateToRoster() { - // `this.router.currentRouteName` is currently `'index'` - // `this.router.queryParams` is currently `{ }` + this.route('faq'); - this.router.transitionTo(`avengers.roster`); - // after the transition, `this.router.queryParams` has restored the value of `{ page: 1 }` - // from the controller state - } - } - ``` + this.route('posts', function() { + this.queryParams('foo', 'baz'); + + this.route('new'); + this.route('index'); + this.route('show'); + }); +}) +``` + +This method of dealing with query params implies the following: + - All query params, even if not specified are allowed. given the above example, I could use the qp "strongestAvenger" anywhere + - Any sort of transition will clear the query params, unless it is defined as a sticky queryParam. So, if I'm on the posts/new route with the query params "foo" and "baz", and transition to posts/show, those query params are still available. but if I navigate to the faq page, foo and baz are cleared from the URL. + - The globally defined query params will stick around until cleared manually. If I visit faq?bar=2, and then transition to posts. the bar=2 query param will still be present. + - The `this.queryParams` function will be available at every nesting of the route tree. + +Notes: + - This does not imply a caching mechanism (like what controllers today allow) + - If a certain app wants caching of query params as they exist today, an addon could be made to fill that gap. The biggest change needed, which could _potentially_ be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. - ### Setting Query Params Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon. @@ -150,14 +154,6 @@ this.router.setQueryParameters({ Just as it is today, setting query params in this way would not trigger a transition. The key-value state of set query params would be available on `#queryParams` as well as an existing controller's computed properties as they exist today. -This doesn't change the existing functionality where objects and arrays are still not correctly serialized to their URI-string counterparts. - -To address that, a long standing issue from as far back as 2016, -some new functionality for serialization and deserialization would be powered by [qs](https://www.npmjs.com/package/qs) ([3.4kb (gzip+min)](https://bundlephobia.com/result?p=qs@6.7.0)) -- which would enable the setting of arrays and objects. - - -Alternatively, [URLSearchParams](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams) is a built-in feature of browsers, and there exists a polyfill for IE11. This might be lighter weight than pulling in qs. - ## How we teach this Currently, query params _must_ be [specified on the controller](https://guides.emberjs.com/release/routing/query-params/): @@ -178,8 +174,6 @@ export default class extends Controller { } ``` -This will no longer be true. However, controllers will still manage the long-term state of the query params as they do today. - Having query-param-related computed properties available everywhere will be a shift in thinking that "the controller manages query params" to "query params are a routing concern and are on the router service" ```ts @@ -205,8 +199,4 @@ export default class ApplicationRoute extends Route { ## Drawbacks - Some people may be relying on the controller query-params allow-list. - -## Alternatives - -React Router, for example, includes the query params as a string with every Route component via the `location` object. -They also provide url segments, which may also be handy, but for now, may be outside the scope of this RFC. +- Some people may be super tied in to controller query params cacheing. From 1a533d8ac3a76682e6330482186f2ddadd8440e2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 8 Aug 2019 22:18:25 -0400 Subject: [PATCH 21/23] Update, add more details, more motivation, more examples --- ...00-router-service-computed-query-params.md | 156 ++++++++++++++++-- 1 file changed, 139 insertions(+), 17 deletions(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 665a38d081..93cb42f831 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -8,8 +8,8 @@ This RFC proposes a new primitive API change to the `RouterService` to allow access to query params from anywhere. -Access to query params is currently restricted to the controller, and subsequently, the corresponding route. -This results in some limitations with which users may consume query param data. +Access to query params is currently restricted to the controller, and subsequently, the corresponding route. +This results in some limitations with which users may consume query param data. By exposing query params on the [RouterService](https://api.emberjs.com/ember/release/classes/RouterService), users will be able to easily access the query params from deep down a component tree, removing the need to pass query param related data and actions down many layers of components. @@ -17,12 +17,16 @@ By exposing query params on the [RouterService](https://api.emberjs.com/ember/re ## Motivation Modern SPA concepts have converged on the idea that query params should be easily accessible -- independent from the object responsible for handling the route. -Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), +Like with the [RouterService](https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md), it is common to have a need to perform routing behavior from deep down a component tree. -Additionally, the current query params implementation feels very verbose for "just wanting to access a property" and has been frustrating to have to explain awkward behavior when on-boarding new devs who may be unfamiliar with Ember. +Additionally, the current query params implementation feels very verbose for "just wanting to access a property" and has been frustrating to have to explain awkward behavior when on-boarding new devs who may be unfamiliar with Ember. Accessing data within the url **should feel easy**. +**What's wrong with the existing query params?** +- For all but one use case, controllers can be avoided. Query Params force controllers into existence for those who are trying to avoid them. +- The caching mechanism is persistent beyond just child routes. _Any_ time a route with query params is re-visited, the query params values will be restored. This can be useful for those who are needing this behavior, but for those who want to manage queryParams via transition / navigations, they'd need to set up query param resets on enter/exit of routes, link-to's and transitions -- the query params implementation becomes more of an obstacle than a feature. + ## Detailed Design ### Accessing Query Params @@ -102,28 +106,109 @@ Router.map(function() { this.route('new'); this.route('index'); - this.route('show'); + this.route( + 'show', + { path: '/:postId', queryParams: ['hideComments', 'invertColors'] }, + function() { + this.route('edit'); + this.route('comment'); + this.route('share'); + } + ); }); -}) +}); ``` This method of dealing with query params implies the following: - All query params, even if not specified are allowed. given the above example, I could use the qp "strongestAvenger" anywhere - - Any sort of transition will clear the query params, unless it is defined as a sticky queryParam. So, if I'm on the posts/new route with the query params "foo" and "baz", and transition to posts/show, those query params are still available. but if I navigate to the faq page, foo and baz are cleared from the URL. + - Any sort of transition will clear the query params, unless it is defined as a sticky queryParam. So, if I'm on the posts/new route with the query params "foo" and "baz", and transition to posts/show, those query params are still available. If I navigate to the faq page, foo and baz are cleared from the URL. - The globally defined query params will stick around until cleared manually. If I visit faq?bar=2, and then transition to posts. the bar=2 query param will still be present. - - The `this.queryParams` function will be available at every nesting of the route tree. + - The `this.queryParams` function will be available at every nesting of the route tree, but `queryParams` are also configurable in the route options hash. -Notes: +Notes: - This does not imply a caching mechanism (like what controllers today allow) - - If a certain app wants caching of query params as they exist today, an addon could be made to fill that gap. + - If a certain app wants caching of query params as they exist today, an addon could be made to fill that gap. + + **A more concrete example: filter query params** +Given we want a way to search over a list of products, be able to view additional information about a selected search result, and save the search result for later, sticky params help maintain the search throughout all of the route navigations (similar to Amazon). + + +```ts +Router.map(function() { + this.queryParams('bar'); + + this.route('search', { queryParams: [ + 'term', 'isPrime', 'department', 'averageReview', 'brand', + 'memoryType', 'processor', 'vRamCapacity', 'certification', + ]}, function() { + // index route will be the search results page + + // shows a selected result with additional information + this.route('summary', { path: '/summary/:itemId' }); + + // shows a modal with a field to name the search to be loaded later + this.route('save'); + }); +}); +``` + +1. visit `/search`. +2. type "RTX" into the search bar press the enter key. +3. the URL now shows `/search?term=RTX` and some results display as rows. + The code for appending `term` may look like the following: + + ```ts + @service router; + + this.router.setQueryParam('term', 'RTX'); + ``` + +4. maybe there is a side panel that has dynamically updated with relevant + potential search values, you check that you want "Prime" only results. +5. the URL now shows `/search?term=RTX&isPrime=true`. +6. clicking on the first result expands the row to fill more of the screen. + The transition to the new route may look like the following: + + ```ts + this.transitionTo('search.summary', '001'); + ``` +7. the URL is now `/search/summary/001?term=RTX&isPrime=true`. + the query params are still present + because the parent route defined sticky query params. +8. clicking "close" on the expanded row transitions back to `/search?term=RTX&isPrime=true` + + ```ts + this.transitionTo('search'); + ``` +9. clicking "save search" opens a modal where you may save your search results. + implementation of the submit action make look like: + ```ts + @service router; + + @action submit() { + let { queryParams } = this.router + + await fetch('some-url', { + method: 'POST', + body: JSON.stringify({ + name: this.name, + search: queryParams + }), + }); + } + + ``` + + +------------------------------------------- The biggest change needed, which could _potentially_ be a breaking change, is that the allow-list on routes' queryParams hash will need to be removed. The controller-based way is static in that all known query params must be specified on the controller ahead of time. This has been a great deal of frustration amongst many developers, both new and old to ember. -This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. +This is a dynamic way to manage query params which hopefully aligns more with people's mental model of query params. ### Setting Query Params -Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon. +Until IE11 support is dropped, we cannot wrap and set query params intuitively as a normal getter/setter as is proposed by this addon. For example, this is not possible until IE11 support is dropped: @@ -133,7 +218,7 @@ this.router.queryParams.strongestAvenger = 'Hulk'; This is due to the fact that IE11 only supports ES5, which [does not have Proxy support](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy#Browser_compatibility) and [cannot be polyfilled](https://babeljs.io/docs/en/learn#proxies). -> Due to the limitations of ES5, Proxies cannot be transpiled or polyfilled. +> Due to the limitations of ES5, Proxies cannot be transpiled or polyfilled. - _[https://babeljs.io/docs/en/learn#proxies](https://babeljs.io/docs/en/learn#proxies) @@ -141,10 +226,10 @@ This is due to the fact that IE11 only supports ES5, which [does not have Proxy Instead, functions must be defined on the router that would take care of the setting of the query param's value. ```ts // set a single parameter -this.router.setQueryParameter('strongestAvenger', 'Captain Marvel'); +this.router.setQueryParam('strongestAvenger', 'Captain Marvel'); // set many parameters -// this'll replace -this.router.setQueryParameters({ +// this'll replace +this.router.setQueryParams({ strongestAvenger: 'Carol Danvers', secondStrongest: 'Thor or Hulk?', ['kebab-case-param']: `kebab o' Thanos' armies`, @@ -154,6 +239,43 @@ this.router.setQueryParameters({ Just as it is today, setting query params in this way would not trigger a transition. The key-value state of set query params would be available on `#queryParams` as well as an existing controller's computed properties as they exist today. +### Interop with Today's Query Params + +Controllers will still need to have their queryParams explicitly set, but for maximum ineteropability: + +**Setting a query param with the new API** +```ts +this.router.setQueryParam('key', 'value'); +``` +The current route is known, and the hierarchy of active controllers are also known. `setQueryParam` could look through the controllers to see if any of them define a query param `key` and then set it. + +**Getting a query param with the new API** +```ts +this.router.queryParams.key; +``` +Simalar to setting the query param, +the getter could also look through the hierarchy of active controllers +to see if a value exists. + +Accessing or Setting query params using the new APIs that happen to collide +with controller-defined query params should throw a warning, +as the interop would give the router's query params an implicit cache, +which may not be present in a future without controllers. + +**Setting a query param with the old API** +```ts +this.set('controllerQp', 'value'); +``` +It's possible to have `set` use `this.router.setQueryParam`, +but it would need to have additional code defined in `set` +which checks if the current instance is a `Controller` +and if the `queryParams` array defines the key. + +**Getting a query param with the old API** +Because this is only possible with query params defined on controllers, +the value of the query param *must* exist on the controller. +There is no need to have compatibility with the router's queryParams here. + ## How we teach this Currently, query params _must_ be [specified on the controller](https://guides.emberjs.com/release/routing/query-params/): @@ -166,7 +288,7 @@ export default class extends Controller { category = null; page = 1; filter = 'recent'; - + @computed('category', 'model') get filteredArticles() { // do something with category and model as category changes From 09f23852fa766f45253da978c07a2a8c44c8c645 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 9 Aug 2019 15:21:49 -0400 Subject: [PATCH 22/23] Update text/0000-router-service-computed-query-params.md Co-Authored-By: Jan Bobisud --- text/0000-router-service-computed-query-params.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 93cb42f831..982431c0ed 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -301,7 +301,7 @@ Having query-param-related computed properties available everywhere will be a sh ```ts import Route from '@ember/routing/route'; import { inject as service } from '@ember/service'; -import { alias } from 'ember-query-params-service'; +import { alias } from '@ember/object/computed'; export default class ApplicationRoute extends Route { @service router; From d42cd00c8233882459c04b8d8b71f9c71f72ce04 Mon Sep 17 00:00:00 2001 From: "L. Preston Sego III" Date: Fri, 23 Aug 2019 16:22:19 -0400 Subject: [PATCH 23/23] Update text/0000-router-service-computed-query-params.md Co-Authored-By: Robert Jackson --- text/0000-router-service-computed-query-params.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-router-service-computed-query-params.md b/text/0000-router-service-computed-query-params.md index 982431c0ed..34311f5cd4 100644 --- a/text/0000-router-service-computed-query-params.md +++ b/text/0000-router-service-computed-query-params.md @@ -59,7 +59,7 @@ Everyone has different query param serialization and deserialization needs depen By default, the query params will be serialized and deserialized via the builtin URLSearchParams API, and polyfilled for IE11. -Should someone decide to customize how serilaization and deserializion transforms the query params, that can be done directly on the router: +Should someone decide to customize how serialization and deserialization transforms the query params, that can be done directly on the router: ```ts import EmberRouter from '@ember/routing/router';