fix(typescript): Tighten up TypeScript settings to be ECMAScript and …

…Deno compatible (#81)
feathersjs · Apr 20, 2021 · 28fe875 · 28fe875
1 parent 6c63cbe
commit 28fe875
Show file tree

Hide file tree

Showing 13 changed files with 724 additions and 1,256 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/hooks/deno/base.ts b/packages/hooks/deno/base.ts
@@ -8,19 +8,22 @@ export type HookContextData = { [key: string]: any };
 /**
  * The base hook context.
  */
-export class HookContext<T = any, C = any> {
-  result?: T;
-  method?: string;
-  self: C;
-  arguments: any[];
+export class BaseHookContext<C = any> {
+  self?: C;
   [key: string]: any;
 
   constructor (data: HookContextData = {}) {
     Object.assign(this, data);
   }
 }
 
-export type HookContextConstructor = new (data?: { [key: string]: any }) => HookContext;
+export interface HookContext<T = any, C = any> extends BaseHookContext<C> {
+  result?: T;
+  method?: string;
+  arguments: any[];
+}
+
+export type HookContextConstructor = new (data?: { [key: string]: any }) => BaseHookContext;
 
 export type HookDefaultsInitializer = (self?: any, args?: any[], context?: HookContext) => HookContextData;
 
@@ -29,9 +32,9 @@ export class HookManager {
   _params: string[]|null = null;
   _middleware: Middleware[]|null = null;
   _props: HookContextData|null = null;
-  _defaults: HookDefaultsInitializer;
+  _defaults?: HookDefaultsInitializer;
 
-  parent (parent: this) {
+  parent (parent: this|null) {
     this._parent = parent;
 
     return this;
@@ -61,7 +64,7 @@ export class HookManager {
       return otherMiddleware.concat(middleware);
     }
 
-    return otherMiddleware || middleware;
+    return otherMiddleware || middleware || [];
   }
 
   props (props: HookContextData) {
@@ -74,14 +77,14 @@ export class HookManager {
     return this;
   }
 
-  getProps (): HookContextData {
+  getProps (): HookContextData|null {
     const previous = this._parent?.getProps();
 
     if (previous && this._props) {
       return copyProperties({}, previous, this._props);
     }
 
-    return previous || this._props;
+    return previous || this._props || null;
   }
 
   params (...params: string[]) {
@@ -90,7 +93,7 @@ export class HookManager {
     return this;
   }
 
-  getParams (): string[] {
+  getParams (): string[]|null {
     const previous = this._parent?.getParams();
 
     if (previous && this._params) {
@@ -106,7 +109,7 @@ export class HookManager {
     return this;
   }
 
-  getDefaults (self: any, args: any[], context: HookContext): HookContextData {
+  getDefaults (self: any, args: any[], context: HookContext): HookContextData|null {
     const defaults = typeof this._defaults === 'function' ? this._defaults(self, args, context) : null;
     const previous = this._parent?.getDefaults(self, args, context);
 
@@ -117,7 +120,7 @@ export class HookManager {
     return previous || defaults;
   }
 
-  getContextClass (Base: HookContextConstructor = HookContext): HookContextConstructor {
+  getContextClass (Base: HookContextConstructor = BaseHookContext): HookContextConstructor {
     const ContextClass = class ContextClass extends Base {
       constructor (data: any) {
         super(data);

diff --git a/packages/hooks/deno/compose.ts b/packages/hooks/deno/compose.ts
@@ -27,7 +27,7 @@ export function compose<T = any> (middleware: Middleware<T>[]) {
 
       index = i;
 
-      let fn = middleware[i];
+      let fn: Middleware|undefined = middleware[i];
 
       if (i === middleware.length) {
         fn = next;

diff --git a/packages/hooks/deno/hooks.ts b/packages/hooks/deno/hooks.ts
@@ -2,7 +2,7 @@ import { compose, Middleware } from './compose.ts';
 import {
   HookContext, setManager, HookContextData, HookOptions, convertOptions, setMiddleware
 } from './base.ts';
-import { copyProperties } from './utils.ts';
+import { copyFnProperties, copyProperties } from './utils.ts';
 
 export function getOriginal (fn: any): any {
   return typeof fn.original === 'function' ? getOriginal(fn.original) : fn;
@@ -51,6 +51,7 @@ export function functionHooks <F> (fn: F, managerOrMiddleware: HookOptions) {
     return compose(hookChain).call(this, context);
   };
 
+  copyFnProperties(wrapper, fn);
   copyProperties(wrapper, fn);
   setManager(wrapper, manager);
 

diff --git a/packages/hooks/deno/index.ts b/packages/hooks/deno/index.ts
@@ -25,6 +25,7 @@ export type MiddlewareOptions = {
 /**
  * Initializes a hook settings object with the given middleware.
  * @param mw The list of middleware
+ * @param options Middleware options (params, default, props)
  */
 export function middleware (mw?: Middleware[], options?: MiddlewareOptions) {
   const manager = new HookManager().middleware(mw);
@@ -55,7 +56,8 @@ export function middleware (mw?: Middleware[], options?: MiddlewareOptions) {
  * (`middleware([]).params()` etc.)
  */
 export function hooks<F, T = any> (
-  fn: F, manager: HookManager
+    fn: F&(() => void),
+    manager?: HookManager
 ): WrappedFunction<F, T>;
 
 /**
@@ -68,17 +70,17 @@ export function hooks<O> (obj: O|(new (...args: any[]) => O), hookMap: HookMap<O
 
 /**
  * Decorate a class method with hooks.
- * @param _manager The hooks settings
+ * @param manager The hooks settings
  */
 export function hooks<T = any> (
-  _manager?: HookOptions
+  manager?: HookOptions
 ): any;
 
 // Fallthrough to actual implementation
 export function hooks (...args: any[]) {
   const [ target, _hooks ] = args;
 
-  if (typeof target === 'function' && (_hooks instanceof HookManager || Array.isArray(_hooks))) {
+  if (typeof target === 'function' && (_hooks instanceof HookManager || Array.isArray(_hooks) || args.length === 1)) {
     return functionHooks(target, _hooks);
   }
 

diff --git a/packages/hooks/deno/utils.ts b/packages/hooks/deno/utils.ts
@@ -37,11 +37,27 @@ export function copyProperties <F> (target: F, ...originals: any[]) {
     for (const prop of originalProps) {
       const propDescriptor = Object.getOwnPropertyDescriptor(original, prop);
 
-      if (!target.hasOwnProperty(prop)) {
+      if (propDescriptor && !Object.prototype.hasOwnProperty.call(target, prop)) {
         Object.defineProperty(target, prop, propDescriptor);
       }
     }
   }
 
   return target;
 }
+
+export function copyFnProperties <F> (target: F, original : any) {
+  const internalProps = ['name', 'length'];
+
+  try {
+    for (const prop of internalProps) {
+      const value = original[prop];
+
+      Object.defineProperty(target, prop, { value });
+    }
+  } catch (e) {
+    // Avoid IE error
+  }
+
+  return target;
+}
diff --git a/packages/hooks/package.json b/packages/hooks/package.json
@@ -34,10 +34,10 @@
     "build": "npm run compile && npm run build:browser",
     "compile": "shx rm -rf lib/ && tsc",
     "prepublish": "npm run build",
-    "test:mocha": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts",
-    "test": "npm run build && npm run test:mocha",
+    "mocha": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts",
+    "test": "npm run build && npm run mocha",
     "build:deno": "shx rm -rf deno && shx mkdir -p deno && node build/deno",
-    "test:deno": "npm run build:deno && deno test --config tsconfig.json test/deno_test.ts"
+    "test:deno": "npm run build:deno && deno test test/deno_test.ts"
   },
   "directories": {
     "lib": "lib"
@@ -55,15 +55,15 @@
     "access": "public"
   },
   "devDependencies": {
-    "@types/chai": "^4.2.15",
+    "@types/chai": "^4.2.16",
     "@types/mocha": "^8.2.2",
-    "@types/node": "^14.14.37",
+    "@types/node": "^14.14.41",
     "mocha": "^8.3.2",
     "shx": "^0.3.3",
-    "ts-loader": "^8.1.0",
+    "ts-loader": "^9.0.1",
     "ts-node": "^9.1.1",
-    "typescript": "^4.2.3",
-    "webpack": "^5.28.0",
+    "typescript": "^4.2.4",
+    "webpack": "^5.34.0",
     "webpack-cli": "^4.6.0"
   }
 }
diff --git a/packages/hooks/src/base.ts b/packages/hooks/src/base.ts
@@ -8,19 +8,22 @@ export type HookContextData = { [key: string]: any };
 /**
  * The base hook context.
  */
-export class HookContext<T = any, C = any> {
-  result?: T;
-  method?: string;
-  self: C;
-  arguments: any[];
+export class BaseHookContext<C = any> {
+  self?: C;
   [key: string]: any;
 
   constructor (data: HookContextData = {}) {
     Object.assign(this, data);
   }
 }
 
-export type HookContextConstructor = new (data?: { [key: string]: any }) => HookContext;
+export interface HookContext<T = any, C = any> extends BaseHookContext<C> {
+  result?: T;
+  method?: string;
+  arguments: any[];
+}
+
+export type HookContextConstructor = new (data?: { [key: string]: any }) => BaseHookContext;
 
 export type HookDefaultsInitializer = (self?: any, args?: any[], context?: HookContext) => HookContextData;
 
@@ -29,9 +32,9 @@ export class HookManager {
   _params: string[]|null = null;
   _middleware: Middleware[]|null = null;
   _props: HookContextData|null = null;
-  _defaults: HookDefaultsInitializer;
+  _defaults?: HookDefaultsInitializer;
 
-  parent (parent: this) {
+  parent (parent: this|null) {
     this._parent = parent;
 
     return this;
@@ -61,7 +64,7 @@ export class HookManager {
       return otherMiddleware.concat(middleware);
     }
 
-    return otherMiddleware || middleware;
+    return otherMiddleware || middleware || [];
   }
 
   props (props: HookContextData) {
@@ -74,14 +77,14 @@ export class HookManager {
     return this;
   }
 
-  getProps (): HookContextData {
+  getProps (): HookContextData|null {
     const previous = this._parent?.getProps();
 
     if (previous && this._props) {
       return copyProperties({}, previous, this._props);
     }
 
-    return previous || this._props;
+    return previous || this._props || null;
   }
 
   params (...params: string[]) {
@@ -90,7 +93,7 @@ export class HookManager {
     return this;
   }
 
-  getParams (): string[] {
+  getParams (): string[]|null {
     const previous = this._parent?.getParams();
 
     if (previous && this._params) {
@@ -106,7 +109,7 @@ export class HookManager {
     return this;
   }
 
-  getDefaults (self: any, args: any[], context: HookContext): HookContextData {
+  getDefaults (self: any, args: any[], context: HookContext): HookContextData|null {
     const defaults = typeof this._defaults === 'function' ? this._defaults(self, args, context) : null;
     const previous = this._parent?.getDefaults(self, args, context);
 
@@ -117,7 +120,7 @@ export class HookManager {
     return previous || defaults;
   }
 
-  getContextClass (Base: HookContextConstructor = HookContext): HookContextConstructor {
+  getContextClass (Base: HookContextConstructor = BaseHookContext): HookContextConstructor {
     const ContextClass = class ContextClass extends Base {
       constructor (data: any) {
         super(data);

diff --git a/packages/hooks/src/compose.ts b/packages/hooks/src/compose.ts
@@ -27,7 +27,7 @@ export function compose<T = any> (middleware: Middleware<T>[]) {
 
       index = i;
 
-      let fn = middleware[i];
+      let fn: Middleware|undefined = middleware[i];
 
       if (i === middleware.length) {
         fn = next;

diff --git a/packages/hooks/src/utils.ts b/packages/hooks/src/utils.ts
@@ -37,7 +37,7 @@ export function copyProperties <F> (target: F, ...originals: any[]) {
     for (const prop of originalProps) {
       const propDescriptor = Object.getOwnPropertyDescriptor(original, prop);
 
-      if (!target.hasOwnProperty(prop)) {
+      if (propDescriptor && !Object.prototype.hasOwnProperty.call(target, prop)) {
         Object.defineProperty(target, prop, propDescriptor);
       }
     }

diff --git a/packages/hooks/test/function.test.ts b/packages/hooks/test/function.test.ts
@@ -4,6 +4,7 @@ import {
   middleware,
   getManager,
   HookContext,
+  BaseHookContext,
   NextFunction,
   setMiddleware,
   functionHooks
@@ -41,7 +42,7 @@ describe('functionHooks', () => {
 
   it('can override arguments, has context', async () => {
     const addYou = async (ctx: HookContext, next: NextFunction) => {
-      assert.ok(ctx instanceof HookContext);
+      assert.ok(ctx instanceof BaseHookContext);
       assert.deepStrictEqual(ctx.arguments, [ 'There' ]);
       ctx.arguments[0] += ' You';
 
@@ -202,8 +203,13 @@ describe('functionHooks', () => {
     };
     const first = hooks(hello, middleware([ one, two ]));
     const second = hooks(first, middleware([ three ]));
+    const mngr = getManager(second);
 
-    assert.deepEqual(getManager(second).getMiddleware(), [ one, two, three ]);
+    if (mngr === null) {
+      assert.ok(false, 'There should be a manager');
+    } else {
+      assert.deepEqual(mngr.getMiddleware(), [ one, two, three ]);
+    }
 
     const result = await second('Dave');
 
@@ -388,8 +394,13 @@ describe('functionHooks', () => {
 
     const customContext = fn.createContext({ message: 'Hi !' });
     const resultContext: HookContext = await fn('Dave', {}, customContext);
+    const keys = Object.keys(resultContext);
 
-    assert.deepEqual(Object.keys(resultContext), ['message', 'name', 'arguments', 'result']);
+    assert.ok(keys.includes('self'));
+    assert.ok(keys.includes('message'));
+    assert.ok(keys.includes('name'));
+    assert.ok(keys.includes('arguments'));
+    assert.ok(keys.includes('result'));
   });
 
   it('same params and props throw an error', async () => {

diff --git a/packages/hooks/tsconfig.json b/packages/hooks/tsconfig.json
@@ -5,7 +5,6 @@
   ],
   "compilerOptions": {
     "outDir": "lib",
-    "experimentalDecorators": true,
-    "strictNullChecks": false
+    "experimentalDecorators": true
   }
 }