feat(context): fix generic typing for BindingFilter

Author: raymondfeng

packages/context/src/binding-filter.ts

@@ -9,9 +9,37 @@ import {BindingAddress} from './binding-key';
 /**
  * A function that filters bindings. It returns `true` to select a given
  * binding.
+ *
+ * TODO(semver-major): We might change this type in the future to either remove
+ * the `<ValueType>` or make it as type guard by asserting the matched binding
+ * to be typed with `<ValueType>`.
+ *
+ * **NOTE**: Originally, we allow filters to be tied with a single value type.
+ * This actually does not make much sense - the filter function is typically
+ * invoked on all bindings to find those ones matching the given criteria.
+ * Filters must be prepared to handle bindings of any value type. We learned
+ * about this problem after enabling TypeScript's `strictFunctionTypes` check,
+ * but decided to preserve `ValueType` argument for backwards compatibility.
+ * The `<ValueType>` represents the value type for matched bindings but it's
+ * not used for checking.
+ *
+ * Ideally, `BindingFilter` should be declared as a type guard as follows:
+ * ```ts
+ * export type BindingFilterGuard<ValueType = unknown> = (
+ *   binding: Readonly<Binding<unknown>>,
+ * ) => binding is Readonly<Binding<ValueType>>;
+ * ```
+ *
+ * But TypeScript treats the following types as incompatible and does not accept
+ * type 1 for type 2.
+ *
+ * 1. `(binding: Readonly<Binding<unknown>>) => boolean`
+ * 2. `(binding: Readonly<Binding<unknown>>) => binding is Readonly<Binding<ValueType>>`
+ *
  */
+// tslint:disable-next-line:no-unused
 export type BindingFilter<ValueType = unknown> = (
-  binding: Readonly<Binding<ValueType>>,
+  binding: Readonly<Binding<unknown>>,
 ) => boolean;
 
 /**

packages/context/src/context-view.ts

@@ -42,7 +42,7 @@ export class ContextView<T = unknown> extends EventEmitter
 
   constructor(
     protected readonly context: Context,
-    public readonly filter: BindingFilter<T>,
+    public readonly filter: BindingFilter,
   ) {
     super();
   }
@@ -161,10 +161,10 @@ export class ContextView<T = unknown> extends EventEmitter
  */
 export function createViewGetter<T = unknown>(
   ctx: Context,
-  bindingFilter: BindingFilter<T>,
+  bindingFilter: BindingFilter,
   session?: ResolutionSession,
 ): Getter<T[]> {
-  const view = new ContextView(ctx, bindingFilter);
+  const view = new ContextView<T>(ctx, bindingFilter);
   view.open();
   return view.asGetter(session);
 }

packages/context/src/context.ts

@@ -39,9 +39,9 @@ if (!Symbol.asyncIterator) {
   (Symbol as any).asyncIterator = Symbol.for('Symbol.asyncIterator');
 }
 /**
- * This import must happen after the polyfill.
- *
- * WARNING: VSCode organize import may change the order of this import
+ * WARNING: This following import must happen after the polyfill. The
+ * `auto-import` by an IDE such as VSCode may move the import before the
+ * polyfill. It must be then fixed manually.
  */
 import {iterator, multiple} from 'p-event';
 
@@ -522,7 +522,7 @@ export class Context extends EventEmitter {
   find<ValueType = BoundValue>(
     pattern?: string | RegExp | BindingFilter,
   ): Readonly<Binding<ValueType>>[] {
-    const bindings: Readonly<Binding>[] = [];
+    const bindings: Readonly<Binding<ValueType>>[] = [];
     const filter = filterByKey(pattern);
 
     for (const b of this.registry.values()) {