Generic type argument for QueryParamGroup

[Airblader]

What's your idea?

It would be great if QueryParamGroup was generic (in terms of the names of the parameters) so that value, valueChanges, setValue and patchValue could be typed better.

I've attempted this in the early days and recall that it wasn't quite that easy, but we should attempt this again.

[Airblader]

I've been playing around with this. It's… really tricky. Christ! So this version (as of commit c7626501be46f8c52d1a9dddef4277b219e8501b) compiles. But plenty of work left to be done here, and it gets ugly. :-/

diff --git a/projects/ngqp/core/src/lib/directives/query-param-group.service.ts b/projects/ngqp/core/src/lib/directives/query-param-group.service.ts
index 80540d3..53e05bd 100644
--- a/projects/ngqp/core/src/lib/directives/query-param-group.service.ts
+++ b/projects/ngqp/core/src/lib/directives/query-param-group.service.ts
@@ -42,7 +42,7 @@ class NavigationData {
 export class QueryParamGroupService implements OnDestroy {

     /** The {@link QueryParamGroup} to bind. */
-    private queryParamGroup: QueryParamGroup;
+    private queryParamGroup: QueryParamGroup<any, any>;

     /** List of {@link QueryParamAccessor} registered to this service. */
     private directives = new Map<string, QueryParamAccessor[]>();
@@ -83,7 +83,7 @@ export class QueryParamGroupService implements OnDestroy {
     /**
      * Uses the given {@link QueryParamGroup} for synchronization.
      */
-    public setQueryParamGroup(queryParamGroup: QueryParamGroup): void {
+    public setQueryParamGroup(queryParamGroup: QueryParamGroup<any, any>): void {
         // FIXME: If this is called when we already have a group, we probably need to do
         //        some cleanup first.
         if (this.queryParamGroup) {
@@ -308,7 +308,7 @@ 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(queryParam: QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown>, value: any): Params {
+    private getParamsForValue(queryParam: QueryParam<any> | MultiQueryParam<any> | PartitionedQueryParam<any>, value: any): Params {
         const partitionedQueryParam = this.wrapIntoPartition(queryParam);
         const partitioned = partitionedQueryParam.partition(value);

@@ -370,8 +370,8 @@ export class QueryParamGroupService implements OnDestroy {
      * Wraps a query parameter into a partition if it isn't already.
      */
     private wrapIntoPartition(
-        queryParam: QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown>
-    ): PartitionedQueryParam<unknown> {
+        queryParam: QueryParam<any> | MultiQueryParam<any> | PartitionedQueryParam<any>
+    ): PartitionedQueryParam<any> {
         if (queryParam instanceof PartitionedQueryParam) {
             return queryParam;
         }
diff --git a/projects/ngqp/core/src/lib/directives/query-param.directive.ts b/projects/ngqp/core/src/lib/directives/query-param.directive.ts
index 1e0111b..3998ae6 100644
--- a/projects/ngqp/core/src/lib/directives/query-param.directive.ts
+++ b/projects/ngqp/core/src/lib/directives/query-param.directive.ts
@@ -32,7 +32,7 @@ export class QueryParamDirective implements QueryParamAccessor, OnChanges, OnDes
     public valueAccessor: ControlValueAccessor | null = null;

     /** @internal */
-    private group = new QueryParamGroup({});
+    private group = new QueryParamGroup<'param', any>({});

     /** @internal */
     constructor(
diff --git a/projects/ngqp/core/src/lib/model/query-param-group.ts b/projects/ngqp/core/src/lib/model/query-param-group.ts
index b5d056e..7639c0c 100644
--- a/projects/ngqp/core/src/lib/model/query-param-group.ts
+++ b/projects/ngqp/core/src/lib/model/query-param-group.ts
@@ -1,8 +1,10 @@
 import { Observable, Subject } from 'rxjs';
 import { isMissing, undefinedToNull } from '../util';
 import { OnChangeFunction } from '../types';
-import { MultiQueryParam, QueryParam, PartitionedQueryParam } from './query-param';
 import { RouterOptions } from '../router-adapter/router-adapter.interface';
+import { MultiQueryParam, QueryParam, PartitionedQueryParam, GroupToParams, GroupKeyToParam } from './query-param';
+
+export const keys = Object.keys as <T>(o: T) => (keyof T)[];

 /**
  * Groups multiple {@link QueryParam} instances to a single unit.
@@ -11,10 +13,10 @@ import { RouterOptions } from '../router-adapter/router-adapter.interface';
  * complete unit. Collecting parameters into a group is required for the synchronization
  * to and from the URL.
  */
-export class QueryParamGroup {
+export class QueryParamGroup<K extends string = string, G extends { [Key in K]: unknown } = any> {

     /** @internal */
-    private readonly _valueChanges = new Subject<Record<string, any>>();
+    private readonly _valueChanges = new Subject<G>();

     /**
      * Emits the values of all parameters in this group whenever at least one changes.
@@ -25,34 +27,26 @@ export class QueryParamGroup {
      *
      * NOTE: This observable does not complete on its own, so ensure to unsubscribe from it.
      */
-    public readonly valueChanges: Observable<Record<string, any>> = this._valueChanges.asObservable();
-
-    /** @internal */
-    private readonly _queryParamAdded$ = new Subject<string>();
+    public readonly valueChanges: Observable<G> = this._valueChanges.asObservable();

     /** @internal */
-    public readonly queryParamAdded$: Observable<string> = this._queryParamAdded$.asObservable();
+    private readonly _queryParamAdded$ = new Subject<K>();

     /** @internal */
-    public readonly queryParams: { [ queryParamName: string ]: QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown> };
+    public readonly queryParamAdded$: Observable<K> = this._queryParamAdded$.asObservable();

-    /** @internal */
-    public readonly routerOptions: RouterOptions;
-
-    private changeFunctions: OnChangeFunction<Record<string, any>>[] = [];
+    private changeFunctions: OnChangeFunction<G>[] = [];

     constructor(
-        queryParams: { [ queryParamName: string ]: QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown> },
-        extras: RouterOptions = {}
+        public readonly queryParams: GroupToParams<K, G>,
+        public readonly routerOptions: RouterOptions = {}
     ) {
-        this.queryParams = queryParams;
-        this.routerOptions = extras;
-
-        Object.values(this.queryParams).forEach(queryParam => queryParam._setParent(this));
+        keys(this.queryParams)
+            .forEach(queryParamName => this.queryParams[ queryParamName ]._setParent(this as any));
     }

     /** @internal */
-    public _registerOnChange(fn: OnChangeFunction<Record<string, any>>): void {
+    public _registerOnChange(fn: OnChangeFunction<G>): void {
         this.changeFunctions.push(fn);
     }

@@ -70,7 +64,9 @@ export class QueryParamGroup {
      *
      * @param queryParamName The name of the parameter instance to retrieve.
      */
-    public get(queryParamName: string): QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown> | null {
+    public get<Key extends K>(
+        queryParamName: Key
+    ): GroupKeyToParam<K, G, Key> | null {
         const param = this.queryParams[ queryParamName ];
         if (!param) {
             return null;
@@ -89,13 +85,16 @@ 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<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown>): void {
+    public add<Key extends K>(
+        queryParamName: Key,
+        queryParam: GroupKeyToParam<K, G, Key>
+    ): void {
         if (this.get(queryParamName)) {
             throw new Error(`A parameter with name ${queryParamName} already exists.`);
         }

         this.queryParams[ queryParamName ] = queryParam;
-        queryParam._setParent(this);
+        queryParam._setParent(this as any);
         this._queryParamAdded$.next(queryParamName);
     }

@@ -108,7 +107,7 @@ export class QueryParamGroup {
      *
      * @param queryParamName The name of the parameter to remove.
      */
-    public remove(queryParamName: string): void {
+    public remove<Key extends K>(queryParamName: Key): void {
         const queryParam = this.get(queryParamName);
         if (!queryParam) {
             throw new Error(`No parameter with name ${queryParamName} found.`);
@@ -125,9 +124,10 @@ export class QueryParamGroup {
      * See {@link QueryParamGroup#valueChanges} for a description of the format of
      * the value.
      */
-    public get value(): Record<string, any> {
-        const value: Record<string, any> = {};
-        Object.keys(this.queryParams).forEach(queryParamName => value[ queryParamName ] = this.queryParams[ queryParamName ].value);
+    public get value(): G {
+        const value = {} as G;
+        keys(this.queryParams)
+            .forEach(queryParamName => value[ queryParamName ] = this.queryParams[ queryParamName ].value);

         return value;
     }
@@ -141,17 +141,17 @@ export class QueryParamGroup {
      * @param value See {@link QueryParamGroup#valueChanges} for a description of the format.
      * @param opts Additional options
      */
-    public patchValue(value: Record<string, any>, opts: {
+    public patchValue(value: Partial<G>, opts: {
         emitEvent?: boolean,
         emitModelToViewChange?: boolean,
     } = {}): void {
-        Object.keys(value).forEach(queryParamName => {
-            const queryParam = this.queryParams[ queryParamName ];
+        keys(value).forEach(queryParamName => {
+            const queryParam = this.queryParams[ queryParamName as K ];
             if (isMissing(queryParam)) {
                 return;
             }

-            queryParam.setValue(value[ queryParamName ], {
+            queryParam.setValue(value[ queryParamName ] as any, {
                 emitEvent: opts.emitEvent,
                 onlySelf: true,
                 emitModelToViewChange: false,
@@ -170,12 +170,12 @@ export class QueryParamGroup {
      * @param value See {@link QueryParamGroup#valueChanges} for a description of the format.
      * @param opts Additional options
      */
-    public setValue(value: Record<string, any>, opts: {
+    public setValue(value: G, opts: {
         emitEvent?: boolean,
         emitModelToViewChange?: boolean,
     } = {}): void {
-        Object.keys(this.queryParams).forEach(queryParamName => {
-            this.queryParams[ queryParamName ].setValue(undefinedToNull(value[ queryParamName ]), {
+        keys(this.queryParams).forEach(queryParamName => {
+            this.queryParams[ queryParamName ].setValue(undefinedToNull(value[ queryParamName ] as any), {
                 emitEvent: opts.emitEvent,
                 onlySelf: true,
                 emitModelToViewChange: false,
diff --git a/projects/ngqp/core/src/lib/model/query-param.ts b/projects/ngqp/core/src/lib/model/query-param.ts
index a32111c..4755567 100644
--- a/projects/ngqp/core/src/lib/model/query-param.ts
+++ b/projects/ngqp/core/src/lib/model/query-param.ts
@@ -288,4 +288,12 @@ export class PartitionedQueryParam<T, G extends unknown[] = unknown[]> extends A
         }
     }

-}
\ No newline at end of file
+}
+
+export type GroupKeyToParam<K extends string, G extends { [P in K]: unknown }, Key extends K> = G[Key] extends (infer U)[]
+    ? (QueryParam<G[Key]> | MultiQueryParam<U> | PartitionedQueryParam<G[Key]>)
+    : QueryParam<G[Key]>;
+
+export type GroupToParams<K extends string, G extends { [P in K]: unknown }> = {
+    [Key in K]?: GroupKeyToParam<K, G, Key>;
+};
\ No newline at end of file
diff --git a/projects/ngqp/core/src/lib/query-param-builder.service.ts b/projects/ngqp/core/src/lib/query-param-builder.service.ts
index 7322297..56e09cb 100644
--- a/projects/ngqp/core/src/lib/query-param-builder.service.ts
+++ b/projects/ngqp/core/src/lib/query-param-builder.service.ts
@@ -9,7 +9,7 @@ import {
 } from './serializers';
 import { LOOSE_IDENTITY_COMPARATOR } from './util';
 import { RouterOptions } from './router-adapter/router-adapter.interface';
-import { MultiQueryParam, QueryParam, PartitionedQueryParam } from './model/query-param';
+import { MultiQueryParam, QueryParam, PartitionedQueryParam, GroupToParams } from './model/query-param';
 import { QueryParamGroup } from './model/query-param-group';
 import { MultiQueryParamOpts, PartitionedQueryParamOpts, QueryParamOpts } from './model/query-param-opts';

@@ -38,10 +38,10 @@ export class QueryParamBuilder {
      * @param extras Additional parameters for this group, overriding global configuration.
      * @returns The new {@link QueryParamGroup}.
      */
-    public group(
-        queryParams: { [ name: string ]: QueryParam<unknown> | MultiQueryParam<unknown> | PartitionedQueryParam<unknown> },
+    public group<K extends string = string, G extends { [Key in K]: unknown } = any>(
+        queryParams: GroupToParams<K, G>,
         extras: RouterOptions = {}
-    ): QueryParamGroup {
+    ): QueryParamGroup<K, G> {
         // TODO Maybe we should first validate that no two queryParams defined the same "param".
         return new QueryParamGroup(queryParams, extras);
     }

[Airblader]

I'd say this is OK if type inference would work out of the box, but it doesn't seem to. I think this may need to be on the backburner for a while longer, perhaps TypeScript will catch up (or someone tells me how to do it properly ;-) )