feat(context): use Readonly to guard immutable values

raymondfeng · raymondfeng · commit 871ddef7644e · 2018-02-26T09:42:49.000-08:00
The built-in Readonly&lt;T&gt; type from TypeScript allows us to guard readonly
usage of objects.

Please note Readonly does not make the object completely immutable, for
example, binding.tags is a Set&lt;string&gt; and tags can still be added/removed
even binding.tags itself is readonly.
diff --git a/packages/context/src/context.ts b/packages/context/src/context.ts
@@ -143,18 +143,20 @@ export class Context {
    * - `*` matches zero or more characters except `.` and `:`
    * - `?` matches exactly one character except `.` and `:`
    */
-  find(pattern?: string | RegExp): Binding[];
+  find(pattern?: string | RegExp): Readonly<Binding>[];
 
   /**
    * Find bindings using a filter function
    * @param filter A function to test on the binding. It returns `true` to
    * include the binding or `false` to exclude the binding.
    */
-  find(filter: (binding: Binding) => boolean): Binding[];
+  find(filter: (binding: Readonly<Binding>) => boolean): Readonly<Binding>[];
 
-  find(pattern?: string | RegExp | ((binding: Binding) => boolean)): Binding[] {
-    let bindings: Binding[] = [];
-    let filter: (binding: Binding) => boolean;
+  find(
+    pattern?: string | RegExp | ((binding: Binding) => boolean),
+  ): Readonly<Binding>[] {
+    let bindings: Readonly<Binding>[] = [];
+    let filter: (binding: Readonly<Binding>) => boolean;
     if (!pattern) {
       filter = binding => true;
     } else if (typeof pattern === 'string') {
@@ -182,13 +184,16 @@ export class Context {
    * - `*` matches zero or more characters except `.` and `:`
    * - `?` matches exactly one character except `.` and `:`
    */
-  findByTag(pattern: string | RegExp): Binding[] {
+  findByTag(pattern: string | RegExp): Readonly<Binding>[] {
     const regexp =
       typeof pattern === 'string' ? this.wildcardToRegExp(pattern) : pattern;
     return this.find(b => Array.from(b.tags).some(t => regexp.test(t)));
   }
 
-  protected _mergeWithParent(childList: Binding[], parentList?: Binding[]) {
+  protected _mergeWithParent(
+    childList: Readonly<Binding>[],
+    parentList?: Readonly<Binding>[],
+  ) {
     if (!parentList) return childList;
     const additions = parentList.filter(parentBinding => {
       // children bindings take precedence
diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts
@@ -23,7 +23,7 @@ const PROPERTIES_KEY = 'inject:properties';
 export interface ResolverFunction {
   (
     ctx: Context,
-    injection: Injection,
+    injection: Readonly<Injection>,
     session?: ResolutionSession,
   ): ValueOrPromise<BoundValue>;
 }
@@ -250,7 +250,7 @@ export namespace inject {
 
 function resolveAsGetter(
   ctx: Context,
-  injection: Injection,
+  injection: Readonly<Injection>,
   session?: ResolutionSession,
 ) {
   // We need to clone the session for the getter as it will be resolved later
@@ -279,9 +279,9 @@ function resolveAsSetter(ctx: Context, injection: Injection) {
 export function describeInjectedArguments(
   target: Object,
   method?: string | symbol,
-): Injection[] {
+): Readonly<Injection>[] {
   method = method || '';
-  const meta = MetadataInspector.getAllParameterMetadata<Injection>(
+  const meta = MetadataInspector.getAllParameterMetadata<Readonly<Injection>>(
     PARAMETERS_KEY,
     target,
     method,
@@ -291,7 +291,7 @@ export function describeInjectedArguments(
 
 function resolveByTag(
   ctx: Context,
-  injection: Injection,
+  injection: Readonly<Injection>,
   session?: ResolutionSession,
 ) {
   const tag: string | RegExp = injection.metadata!.tag;
@@ -311,9 +311,9 @@ function resolveByTag(
  */
 export function describeInjectedProperties(
   target: Object,
-): MetadataMap<Injection> {
+): MetadataMap<Readonly<Injection>> {
   const metadata =
-    MetadataInspector.getAllPropertyMetadata<Injection>(
+    MetadataInspector.getAllPropertyMetadata<Readonly<Injection>>(
       PROPERTIES_KEY,
       target,
     ) || {};
diff --git a/packages/context/src/resolution-session.ts b/packages/context/src/resolution-session.ts
@@ -24,15 +24,15 @@ export type ResolutionAction = (
  */
 export interface BindingElement {
   type: 'binding';
-  value: Binding;
+  value: Readonly<Binding>;
 }
 
 /**
  * Wrapper for injections tracked by resolution sessions
  */
 export interface InjectionElement {
   type: 'injection';
-  value: Injection;
+  value: Readonly<Injection>;
 }
 
 /**
@@ -90,7 +90,7 @@ export class ResolutionSession {
    * @param session The current resolution session
    */
   private static enterBinding(
-    binding: Binding,
+    binding: Readonly<Binding>,
     session?: ResolutionSession,
   ): ResolutionSession {
     session = session || new ResolutionSession();
@@ -106,7 +106,7 @@ export class ResolutionSession {
    */
   static runWithBinding(
     action: ResolutionAction,
-    binding: Binding,
+    binding: Readonly<Binding>,
     session?: ResolutionSession,
   ) {
     const resolutionSession = ResolutionSession.enterBinding(binding, session);
@@ -122,7 +122,7 @@ export class ResolutionSession {
    * @param session The current resolution session
    */
   private static enterInjection(
-    injection: Injection,
+    injection: Readonly<Injection>,
     session?: ResolutionSession,
   ): ResolutionSession {
     session = session || new ResolutionSession();
@@ -138,7 +138,7 @@ export class ResolutionSession {
    */
   static runWithInjection(
     action: ResolutionAction,
-    injection: Injection,
+    injection: Readonly<Injection>,
     session?: ResolutionSession,
   ) {
     const resolutionSession = ResolutionSession.enterInjection(
@@ -155,7 +155,7 @@ export class ResolutionSession {
    * Describe the injection for debugging purpose
    * @param injection Injection object
    */
-  static describeInjection(injection?: Injection) {
+  static describeInjection(injection?: Readonly<Injection>) {
     /* istanbul ignore if */
     if (injection == null) return undefined;
     const name = getTargetName(
@@ -175,7 +175,7 @@ export class ResolutionSession {
    * Push the injection onto the session
    * @param injection Injection The current injection
    */
-  pushInjection(injection: Injection) {
+  pushInjection(injection: Readonly<Injection>) {
     /* istanbul ignore if */
     if (debugSession.enabled) {
       debugSession(
@@ -214,7 +214,7 @@ export class ResolutionSession {
   /**
    * Getter for the current injection
    */
-  get currentInjection(): Injection | undefined {
+  get currentInjection(): Readonly<Injection> | undefined {
     for (let i = this.stack.length - 1; i >= 0; i--) {
       const element = this.stack[i];
       if (isInjection(element)) return element.value;
@@ -225,7 +225,7 @@ export class ResolutionSession {
   /**
    * Getter for the current binding
    */
-  get currentBinding(): Binding | undefined {
+  get currentBinding(): Readonly<Binding> | undefined {
     for (let i = this.stack.length - 1; i >= 0; i--) {
       const element = this.stack[i];
       if (isBinding(element)) return element.value;
@@ -237,7 +237,7 @@ export class ResolutionSession {
    * Enter the resolution of the given binding. If
    * @param binding Binding
    */
-  pushBinding(binding: Binding) {
+  pushBinding(binding: Readonly<Binding>) {
     /* istanbul ignore if */
     if (debugSession.enabled) {
       debugSession('Enter binding:', binding.toJSON());
@@ -260,7 +260,7 @@ export class ResolutionSession {
   /**
    * Exit the resolution of a binding
    */
-  popBinding() {
+  popBinding(): Readonly<Binding> {
     const top = this.stack.pop();
     if (!isBinding(top)) {
       throw new Error('The top element must be a binding');
@@ -277,14 +277,14 @@ export class ResolutionSession {
   /**
    * Getter for bindings on the stack
    */
-  get bindingStack(): Binding[] {
+  get bindingStack(): Readonly<Binding>[] {
     return this.stack.filter(isBinding).map(e => e.value);
   }
 
   /**
    * Getter for injections on the stack
    */
-  get injectionStack(): Injection[] {
+  get injectionStack(): Readonly<Injection>[] {
     return this.stack.filter(isInjection).map(e => e.value);
   }
 
diff --git a/packages/context/src/resolver.ts b/packages/context/src/resolver.ts
@@ -111,7 +111,7 @@ export function instantiateClass<T>(
  */
 function resolve<T>(
   ctx: Context,
-  injection: Injection,
+  injection: Readonly<Injection>,
   session?: ResolutionSession,
 ): ValueOrPromise<T> {
   /* istanbul ignore if */
diff --git a/packages/context/test/unit/context.ts b/packages/context/test/unit/context.ts
@@ -202,7 +202,7 @@ describe('Context', () => {
     });
 
     it('escapes reserved chars for regexp', () => {
-      const b1 = ctx.bind('foo');
+      ctx.bind('foo');
       const b2 = ctx.bind('foo+bar');
       const b3 = ctx.bind('foo|baz');
       let result = ctx.find('fo+');
