fix(core): Improve typing and support emptyOn for multi:true

This change improves the type safety by not misinterpreting an
array-valued parameter as a multi: true parameter.

As a side effect we can now support emptyOn for multi: true parameters.

fixes #92
fixes #27

Signed-off-by: Ingo Bürk <ingo.buerk@tngtech.com>

Author: Airblader

projects/ngqp-demo/src/app/docs-items/configuration/query-param/query-param-configuration-docs.component.html

@@ -29,6 +29,10 @@ <h2>Components with multiple values</h2>
         <span apiDocsLink="QueryParam#multi">multi</span> option to <code>true</code>. This will use the default
         behavior of the router to have query parameters with multiple values.
     </p>
+    <div class="alert alert-info">
+        When using <code>multi: true</code>, APIs such as <span apiDocsLink>QueryParamGroup#get</span> will actually
+        return <span apiDocsLink>MultiQueryParam</span> instead of <span apiDocsLink>QueryParam</span>.
+    </div>
     <demo-multi-example></demo-multi-example>
 
     <docs-fragment fragment="debounceTime">
@@ -54,9 +58,6 @@ <h2>Debouncing user input</h2>
     <docs-fragment fragment="emptyOn-compareWith">
         <h2>Default values</h2>
     </docs-fragment>
-    <div class="alert alert-warning">
-        This feature is currently not supported if you are using <span apiDocsLink>QueryParam#multi</span>.
-    </div>
     <p>
         By default, if a parameter value is <code>null</code>, the parameter will be removed from the URL. For example,
         this prevents useless URL segments such as <code>?q=</code> (without a value). However, in some cases you might

projects/ngqp-demo/src/app/docs-items/examples/manual-wiring-example/manual-wiring-example.component.ts

@@ -29,11 +29,10 @@ export class ManualWiringExampleComponent implements OnDestroy {
         this.pageParam.valueChanges.pipe(
             takeUntil(this.componentDestroyed$)
         ).subscribe(page => this.currentPage = page);
-
     }
 
     public get pageParam(): QueryParam<number> {
-        return this.paramGroup.get('page');
+        return this.paramGroup.get('page') as QueryParam<number>;
     }
 
     public onPageChange(page: number) {

projects/ngqp-demo/src/app/docs-items/programmatic-access/query-param-group/query-param-group-programmatic-access-docs.component.html

@@ -5,7 +5,7 @@ <h2>Accessing parameters</h2>
     <p>
         To get access to a specific <span apiDocsLink>QueryParam</span> use the
         <span apiDocsLink>QueryParamGroup#get</span> method by passing the name of the parameter to it. This will
-        return the appropriate <span apiDocsLink>QueryParam</span> instance.
+        return the appropriate <span apiDocsLink>QueryParam</span> or <span apiDocsLink>MultiQueryParam</span> instance.
     </p>
     <div class="alert alert-info">
         Remember that this refers to the name with which the parameter is associated in its group, not the URL parameter

projects/ngqp-demo/src/app/docs-items/programmatic-access/query-param-group/snippets/qpg-api-get.example.ts

@@ -11,7 +11,7 @@ export class ExampleComponent {
             searchText: qpb.stringParam('q'),
         });
 
-        const searchText: QueryParam<string> = this.paramGroup.get('searchText');
+        const searchText: QueryParam<string> = this.paramGroup.get('searchText') as QueryParam<string>;
     }
 
 }

projects/ngqp-demo/src/app/shared/docs-page.pipes.ts

@@ -16,15 +16,15 @@ export class DocsPageNamePipe implements PipeTransform {
             case DocsPage.GETTING_HELP:
                 return 'Getting Help';
             case DocsPage.CONFIGURATION_QUERYPARAMMODULE:
-                return 'QueryParamModule';
+                return 'Global';
             case DocsPage.CONFIGURATION_QUERYPARAMGROUP:
-                return 'QueryParamGroup';
+                return 'Groups';
             case DocsPage.CONFIGURATION_QUERYPARAM:
-                return 'QueryParam';
+                return 'Parameters';
             case DocsPage.PROGRAMMATIC_QUERYPARAMGROUP:
-                return 'QueryParamGroup';
+                return 'Groups';
             case DocsPage.PROGRAMMATIC_QUERYPARAM:
-                return 'QueryParam';
+                return 'Parameters';
             case DocsPage.CUSTOM_CONTROL_VALUE_ACCESSOR:
                 return 'Controls without ControlValueAccessor';
             default:

projects/ngqp/core/src/lib/core.ts

@@ -2,8 +2,6 @@ export { QueryParamBuilder } from './query-param-builder.service';
 export { QueryParamModule } from './query-param.module';
 
 export {
-    createEmptyOnSerializer,
-    createEmptyOnDeserializer,
     createStringSerializer,
     createStringDeserializer,
     createNumberSerializer,

projects/ngqp/core/src/lib/directives/query-param-group.service.ts

@@ -14,27 +14,16 @@ import {
     tap
 } from 'rxjs/operators';
 import { compareParamMaps, filterParamMap, isMissing, isPresent, NOP } from '../util';
-import { Unpack } from '../types';
 import { QueryParamGroup } from '../model/query-param-group';
-import { QueryParam } from '../model/query-param';
+import { MultiQueryParam, QueryParam } from '../model/query-param';
 import { NGQP_ROUTER_ADAPTER, NGQP_ROUTER_OPTIONS, RouterAdapter, RouterOptions } from '../router-adapter/router-adapter.interface';
 import { QueryParamAccessor } from './query-param-accessor.interface';
 
 /** @internal */
-function isMultiQueryParam<T>(queryParam: QueryParam<T> | QueryParam<T[]>): queryParam is QueryParam<T[]> {
+function isMultiQueryParam<T>(queryParam: QueryParam<T> | MultiQueryParam<T>): queryParam is MultiQueryParam<T> {
     return queryParam.multi;
 }
 
-/** @internal */
-function hasArrayValue<T>(queryParam: QueryParam<T> | QueryParam<T[]>, value: T | T[]): value is T[] {
-    return isMultiQueryParam(queryParam);
-}
-
-/** @internal */
-function hasArraySerialization(queryParam: QueryParam<any>, values: string | string[] | null): values is string[] {
-    return isMultiQueryParam(queryParam);
-}
-
 /** @internal */
 class NavigationData {
     constructor(public params: Params, public synthetic: boolean = false) {
@@ -113,7 +102,7 @@ export class QueryParamGroupService implements OnDestroy {
         // it as it might change over time.
         const queryParamName = directive.name;
 
-        const queryParam: QueryParam<any> = this.queryParamGroup.get(queryParamName);
+        const queryParam = this.queryParamGroup.get(queryParamName);
         if (!queryParam) {
             throw new Error(`Could not find query param with name ${queryParamName}. Did you forget to add it to your QueryParamGroup?`);
         }
@@ -160,7 +149,7 @@ export class QueryParamGroupService implements OnDestroy {
         });
 
         this.directives.delete(queryParamName);
-        const queryParam: QueryParam<any> = this.queryParamGroup.get(queryParamName);
+        const queryParam = this.queryParamGroup.get(queryParamName);
         if (queryParam) {
             queryParam._clearChangeFunctions();
         }
@@ -179,7 +168,7 @@ export class QueryParamGroupService implements OnDestroy {
         this.queryParamGroup._registerOnChange((newValue: Record<string, any>) => {
             let params: Params = {};
             Object.keys(newValue).forEach(queryParamName => {
-                const queryParam: QueryParam<any> = this.queryParamGroup.get(queryParamName);
+                const queryParam = this.queryParamGroup.get(queryParamName);
                 if (isMissing(queryParam)) {
                     return;
                 }
@@ -198,7 +187,7 @@ export class QueryParamGroupService implements OnDestroy {
     }
 
     private setupParamChangeListener(queryParamName: string): void {
-        const queryParam: QueryParam<any> = this.queryParamGroup.get(queryParamName);
+        const queryParam = this.queryParamGroup.get(queryParamName);
         if (!queryParam) {
             throw new Error(`No param in group found for name ${queryParamName}`);
         }
@@ -232,10 +221,10 @@ export class QueryParamGroupService implements OnDestroy {
             const groupValue: Record<string, any> = {};
 
             Object.keys(this.queryParamGroup.queryParams).forEach(queryParamName => {
-                const queryParam: QueryParam<any> = this.queryParamGroup.get(queryParamName);
-                const newValue = queryParam.multi
-                    ? this.deserialize(queryParam, queryParamMap.getAll(queryParam.urlParam))
-                    : this.deserialize(queryParam, queryParamMap.get(queryParam.urlParam));
+                const queryParam = this.queryParamGroup.get(queryParamName);
+                const newValue = isMultiQueryParam(queryParam)
+                    ? queryParam.deserializeValue(queryParamMap.getAll(queryParam.urlParam))
+                    : queryParam.deserializeValue(queryParamMap.get(queryParam.urlParam));
 
                 const directives = this.directives.get(queryParamName);
                 if (directives) {
@@ -310,8 +299,8 @@ export class QueryParamGroupService implements OnDestroy {
      * This consists mainly of properly serializing the model value and ensuring to take
      * side effect changes into account that may have been configured.
      */
-    private getParamsForValue<T>(queryParam: QueryParam<any>, value: T | undefined | null): Params {
-        const newValue = this.serialize(queryParam, value);
+    private getParamsForValue(queryParam: QueryParam<any> | MultiQueryParam<any>, value: any | undefined | null): Params {
+        const newValue = queryParam.serializeValue(value);
 
         const combinedParams: Params = isMissing(queryParam.combineWith)
             ? {} : queryParam.combineWith(value);
@@ -324,22 +313,6 @@ export class QueryParamGroupService implements OnDestroy {
         };
     }
 
-    private serialize<T>(queryParam: QueryParam<any>, value: T): string | string[] {
-        if (hasArrayValue(queryParam, value)) {
-            return (value || []).map(queryParam.serialize);
-        } else {
-            return queryParam.serialize(value);
-        }
-    }
-
-    private deserialize<T>(queryParam: QueryParam<T>, values: string | string[]): Unpack<T> | Unpack<T>[] {
-        if (hasArraySerialization(queryParam, values)) {
-            return values.map(queryParam.deserialize);
-        } else {
-            return queryParam.deserialize(values);
-        }
-    }
-
     /**
      * Returns the current set of options to pass to the router.
      *

projects/ngqp/core/src/lib/model/model.ts

@@ -1,3 +1,3 @@
-export { QueryParamOpts } from './query-param-opts';
+export { QueryParamOpts, MultiQueryParamOpts } from './query-param-opts';
 export { QueryParamGroup } from './query-param-group';
-export { QueryParam } from './query-param';
+export { AbstractQueryParam, QueryParam, MultiQueryParam } from './query-param';

projects/ngqp/core/src/lib/model/query-param-group.spec.ts

@@ -46,7 +46,7 @@ describe(QueryParamGroup.name, () => {
         let group: QueryParamGroup;
         let queryParam: QueryParam<string>;
         beforeEach(() => group = new QueryParamGroup({ q: stringParam }));
-        beforeEach(() => queryParam = group.get('q'));
+        beforeEach(() => queryParam = group.get('q') as QueryParam<string> );
 
         it('updates the parameter value', () => {
             group.patchValue({ q: 'Test' });
@@ -114,7 +114,7 @@ describe(QueryParamGroup.name, () => {
         let group: QueryParamGroup;
         let queryParam: QueryParam<string>;
         beforeEach(() => group = new QueryParamGroup({ q: stringParam }));
-        beforeEach(() => queryParam = group.get('q'));
+        beforeEach(() => queryParam = group.get('q') as QueryParam<string>);
 
         it('updates the parameter value', () => {
             group.setValue({ q: 'Test' });
@@ -178,7 +178,7 @@ describe(QueryParamGroup.name, () => {
         let group: QueryParamGroup;
         let queryParam: QueryParam<string>;
         beforeEach(() => group = new QueryParamGroup({ q: stringParam }));
-        beforeEach(() => queryParam = group.get('q'));
+        beforeEach(() => queryParam = group.get('q') as QueryParam<string>);
 
         it('emits the current value', fakeAsync(() => {
             scheduler.run(({ expectObservable }) => {

projects/ngqp/core/src/lib/model/query-param-group.ts

@@ -1,7 +1,7 @@
 import { Observable, Subject } from 'rxjs';
 import { isMissing, undefinedToNull } from '../util';
 import { OnChangeFunction } from '../types';
-import { QueryParam } from './query-param';
+import { MultiQueryParam, QueryParam } from './query-param';
 import { RouterOptions } from '../router-adapter/router-adapter.interface';
 
 /**
@@ -34,15 +34,15 @@ export class QueryParamGroup {
     public readonly queryParamAdded$: Observable<string> = this._queryParamAdded$.asObservable();
 
     /** @internal */
-    public readonly queryParams: { [ queryParamName: string ]: QueryParam<any> };
+    public readonly queryParams: { [ queryParamName: string ]: QueryParam<any> | MultiQueryParam<any> };
 
     /** @internal */
     public readonly routerOptions: RouterOptions;
 
     private changeFunctions: OnChangeFunction<Record<string, any>>[] = [];
 
     constructor(
-        queryParams: { [ queryParamName: string ]: QueryParam<any> },
+        queryParams: { [ queryParamName: string ]: QueryParam<any> | MultiQueryParam<any> },
         extras: RouterOptions = {}
     ) {
         this.queryParams = queryParams;
@@ -69,7 +69,7 @@ export class QueryParamGroup {
      *
      * @param queryParamName The name of the parameter instance to retrieve.
      */
-    public get(queryParamName: string): QueryParam<any> | null {
+    public get(queryParamName: string): QueryParam<any> | MultiQueryParam<any> | null {
         const param = this.queryParams[ queryParamName ];
         if (!param) {
             return null;
@@ -88,7 +88,7 @@ export class QueryParamGroup {
      * @param queryParamName Name of the parameter to reference it with.
      * @param queryParam The new parameter to add.
      */
-    public add(queryParamName: string, queryParam: QueryParam<any>): void {
+    public add(queryParamName: string, queryParam: QueryParam<any> | MultiQueryParam<any>): void {
         if (this.get(queryParamName)) {
             throw new Error(`A parameter with name ${queryParamName} already exists.`);
         }
@@ -107,7 +107,7 @@ export class QueryParamGroup {
      * @param queryParamName The name of the parameter to remove.
      */
     public remove(queryParamName: string): void {
-        const queryParam: QueryParam<any> = this.get(queryParamName);
+        const queryParam = this.get(queryParamName);
         if (!queryParam) {
             throw new Error(`No parameter with name ${queryParamName} found.`);
         }