Skip to content

Commit

Permalink
refactor: enable type checks inside mixin classes
Browse files Browse the repository at this point in the history
Introduce a new type helper `MixinTarget` allowing mixin functions
to accept a type that describes public members of the target class
only. This is working around the current TypeScript limitations.

Rework all existing mixins and the related documentation to use
`MixinTarget<RealClass>` instead of `Constructor<any>`.

Fix any errors discovered by the compiler after enabling type checks.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
  • Loading branch information
bajtos committed May 12, 2020
1 parent bcd033f commit 4c03819
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 57 deletions.
2 changes: 1 addition & 1 deletion docs/site/Creating-components.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ The following snippet is an abbreviated function
{% include code-caption.html content="src/mixins/repository.mixin.ts" %}

```ts
export function RepositoryMixin<T extends Class<any>>(superClass: T) {
export function RepositoryMixin<T extends MixinTarget<Application>>(superClass: T) {
return class extends superClass {
constructor(...args: any[]) {
super(...args);
Expand Down
4 changes: 2 additions & 2 deletions docs/site/Mixin.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Define mixin `TimeStampMixin`:
```ts
import {Class} from '@loopback/repository';

export function TimeStampMixin<T extends Class<any>>(baseClass: T) {
export function TimeStampMixin<T extends MixinTarget<Object>>(baseClass: T) {
return class extends baseClass {
// add a new property `createdAt`
public createdAt: Date;
Expand All @@ -76,7 +76,7 @@ And define mixin `LoggerMixin`:
```ts
import {Class} from '@loopback/repository';

function LoggerMixin<T extends Class<any>>(baseClass: T) {
function LoggerMixin<T extends MixinTarget<Object>>(baseClass: T) {
return class extends baseClass {
// add a new method `log()`
log(str: string) {
Expand Down
2 changes: 1 addition & 1 deletion docs/site/Testing-Your-Extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class. Following is an example for an integration test for a Mixin:

```ts
import {Constructor} from '@loopback/context';
export function TimeMixin<T extends Constructor<any>>(superClass: T) {
export function TimeMixin<T extends MixinTarget<Object>>(superClass: T) {
return class extends superClass {
constructor(...args: any[]) {
super(...args);
Expand Down
14 changes: 8 additions & 6 deletions docs/site/migration/models/mixins.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ This document will guide you in migrating custom model mixins, and custom
method/remote method mixins in LoopBack 3 to their equivalent implementations in
LoopBack 4.

For an understanding of how models in LoopBack 3 are now architectually
For an understanding of how models in LoopBack 3 are now architecturally
decoupled into 3 classes (model, repository, and controller) please read
[Migrating custom model methods](./methods.md).

Expand Down Expand Up @@ -169,7 +169,7 @@ import {property, Model} from '@loopback/repository';
* @param superClass - Base Class
* @typeParam T - Model class
*/
export function AddCategoryPropertyMixin<T extends Constructor<Model>>(
export function AddCategoryPropertyMixin<T extends MixinTarget<Model>>(
superClass: T,
) {
class MixedModel extends superClass {
Expand Down Expand Up @@ -447,7 +447,7 @@ import {FindByTitle} from './find-by-title-interface';
*/
export function FindByTitleRepositoryMixin<
M extends Model & {title: string},
R extends Constructor<CrudRepository<M>>
R extends MixinTarget<CrudRepository<M>>
>(superClass: R) {
class MixedRepository extends superClass implements FindByTitle<M> {
async findByTitle(title: string): Promise<M[]> {
Expand Down Expand Up @@ -552,10 +552,12 @@ export interface FindByTitleControllerMixinOptions {
*/
export function FindByTitleControllerMixin<
M extends Model,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
T extends Constructor<any> = Constructor<object>
T extends MixinTarget<object>
>(superClass: T, options: FindByTitleControllerMixinOptions) {
class MixedController extends superClass implements FindByTitle<M> {
// Value will be provided by the subclassed controller class
repository: FindByTitle<M>;

@get(`${options.basePath}/findByTitle/{title}`, {
responses: {
'200': {
Expand Down Expand Up @@ -587,7 +589,7 @@ mixin class factory function needs to accept some options. We defined an
interface `FindByTitleControllerMixinOptions` to allow for this.

It is also a good idea to give the injected repository (in the controller super
class) a generic name like `this.respository` to keep things simple in the mixin
class) a generic name like `this.repository` to keep things simple in the mixin
class factory function.

#### Generating A Controller Via The CLI
Expand Down
5 changes: 2 additions & 3 deletions examples/log-extension/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,11 @@ providing it via `ApplicationOptions` or using a helper method
`app.logLevel(level: number)`.

```ts
import {Constructor} from '@loopback/context';
import {MixinTarget, Application} from '@loopback/core';
import {EXAMPLE_LOG_BINDINGS} from '../keys';
import {LogComponent} from '../component';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function LogMixin<T extends Constructor<any>>(superClass: T) {
export function LogMixin<T extends MixinTarget<Application>>(superClass: T) {
return class extends superClass {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
constructor(...args: any[]) {
Expand Down
7 changes: 3 additions & 4 deletions examples/log-extension/src/mixins/log.mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Constructor} from '@loopback/context';
import {EXAMPLE_LOG_BINDINGS, LOG_LEVEL} from '../keys';
import {Application, MixinTarget} from '@loopback/core';
import {LogComponent} from '../component';
import {EXAMPLE_LOG_BINDINGS, LOG_LEVEL} from '../keys';

/**
* A mixin class for Application that can bind logLevel from `options`.
Expand All @@ -17,8 +17,7 @@ import {LogComponent} from '../component';
* class MyApplication extends LogMixin(Application) {}
* ```
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function LogMixin<T extends Constructor<any>>(superClass: T) {
export function LogMixin<T extends MixinTarget<Application>>(superClass: T) {
return class extends superClass {
// A mixin class has to take in a type any[] argument!
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
58 changes: 38 additions & 20 deletions packages/boot/src/mixins/boot.mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

import {
Binding,
BindingFromClassOptions,
BindingScope,
Constructor,
Context,
createBindingFromClass,
} from '@loopback/context';
import {Application, Component, MixinTarget} from '@loopback/core';
import {BootComponent} from '../boot.component';
import {Bootstrapper} from '../bootstrapper';
import {BootBindings, BootTags} from '../keys';
Expand All @@ -30,18 +32,8 @@ export {Binding};
* - Provides the `booter()` convenience method to bind a Booter(s) to the Application
* - Override `component()` to call `mountComponentBooters`
* - Adds `mountComponentBooters` which binds Booters to the application from `component.booters[]`
*
* ******************** NOTE ********************
* Trying to constrain the type of this Mixin (or any Mixin) will cause errors.
* For example, constraining this Mixin to type Application require all types using by
* Application to be imported (including it's dependencies such as ResolutionSession).
* Another issue was that if a Mixin that is type constrained is used with another Mixin
* that is not, it will result in an error.
* Example (class MyApp extends BootMixin(RepositoryMixin(Application))) {};
********************* END OF NOTE ********************
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function BootMixin<T extends Constructor<any>>(superClass: T) {
export function BootMixin<T extends MixinTarget<Application>>(superClass: T) {
return class extends superClass implements Bootable {
projectRoot: string;
bootOptions?: BootOptions;
Expand All @@ -57,26 +49,42 @@ export function BootMixin<T extends Constructor<any>>(superClass: T) {
() => this.projectRoot,
);
this.bind(BootBindings.BOOT_OPTIONS).toDynamicValue(
() => this.bootOptions,
() => this.bootOptions ?? {},
);
}

/**
* Convenience method to call bootstrapper.boot() by resolving bootstrapper
*/
async boot(): Promise<void> {
if (this.state === 'booting') return this.awaitState('booted');
this.assertNotInProcess('boot');
this.assertInStates('boot', 'created', 'booted');
/* eslint-disable @typescript-eslint/ban-ts-ignore */
// A workaround to access protected Application methods
const self = (this as unknown) as Application;

if (this.state === 'booting') {
// @ts-ignore
return self.awaitState('booted');
}
// @ts-ignore
self.assertNotInProcess('boot');
// @ts-ignore
self.assertInStates('boot', 'created', 'booted');

if (this.state === 'booted') return;
this.setState('booting');
// @ts-ignore
self.setState('booting');

// Get a instance of the BootStrapper
const bootstrapper: Bootstrapper = await this.get(
BootBindings.BOOTSTRAPPER_KEY,
);

await bootstrapper.boot();

// @ts-ignore
this.setState('booted');

/* eslint-enable @typescript-eslint/ban-ts-ignore */
}

/**
Expand Down Expand Up @@ -115,9 +123,17 @@ export function BootMixin<T extends Constructor<any>>(superClass: T) {
* app.component(ProductComponent);
* ```
*/
public component(component: Constructor<{}>) {
super.component(component);
this.mountComponentBooters(component);
// Unfortunately, TypeScript does not allow overriding methods inherited
// from mapped types. https://github.com/microsoft/TypeScript/issues/38496
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore
public component<C extends Component = Component>(
componentCtor: Constructor<C>,
nameOrOptions?: string | BindingFromClassOptions,
) {
const binding = super.component(componentCtor, nameOrOptions);
this.mountComponentBooters(componentCtor);
return binding;
}

/**
Expand All @@ -129,7 +145,9 @@ export function BootMixin<T extends Constructor<any>>(superClass: T) {
*/
mountComponentBooters(component: Constructor<{}>) {
const componentKey = `components.${component.name}`;
const compInstance = this.getSync(componentKey);
const compInstance = this.getSync<{
booters?: Constructor<Booter>[];
}>(componentKey);

if (compInstance.booters) {
this.booters(...compInstance.booters);
Expand Down
70 changes: 70 additions & 0 deletions packages/core/src/core.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright IBM Corp. 2017,2020. All Rights Reserved.
// Node module: @loopback/core
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Constructor} from '@loopback/context';

/**
* A replacement for `typeof Target` to be used in mixin class definitions.
* This is a workaround for TypeScript limitation described in
* - https://github.com/microsoft/TypeScript/issues/17293
* - https://github.com/microsoft/TypeScript/issues/17744
* - https://github.com/microsoft/TypeScript/issues/36060
*
* @example
*
* ```ts
* export function MyMixin<T extends MixinTarget<Application>>(superClass: T) {
* return class extends superClass {
* // contribute new class members
* }
* };
* ```
*
* TypeScript does not allow class mixins to access protected members from
* the base class. You can use the following approach as a workaround:
*
* ```ts
* // @ts-ignore
* (this as unknown as {YourBaseClass}).protectedMember
* ```
*
* The directive `@ts-ignore` suppresses compiler error about accessing
* a protected member from outside. Unfortunately, it also disables other
* compile-time checks (e.g. to verify that a protected method was invoked
* with correct arguments, and so on). This is the same behavior you
* would get by using `Constructor<any>` instead of `MixinTarget<Application>`.
* The major improvement is that TypeScript can still infer the return
* type of the protected member, therefore `any` is NOT introduced to subsequent
* code.
*
* TypeScript also does not allow mixin class to overwrite a method inherited
* from a mapped type, see https://github.com/microsoft/TypeScript/issues/38496
* As a workaround, use `@ts-ignore` to disable the error.
*
* ```ts
* export function RepositoryMixin<T extends MixinTarget<Application>>(
* superClass: T,
* ) {
* return class extends superClass {
* // @ts-ignore
* public component<C extends Component = Component>(
* componentCtor: Constructor<C>,
* nameOrOptions?: string | BindingFromClassOptions,
* ) {
* const binding = super.component(componentCtor, nameOrOptions);
* // ...
* return binding;
* }
* }
* ```
*/
export type MixinTarget<T extends object> = Constructor<
{
// Enumerate only public members to avoid the following compiler error:
// Property '(name)' of exported class expression
// may not be private or protected.ts(4094)
[p in keyof T]: T[p];
}
>;
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export * from '@loopback/context';
// Export APIs
export * from './application';
export * from './component';
export * from './core.types';
export * from './extension-point';
export * from './keys';
export * from './lifecycle';
Expand Down
3 changes: 1 addition & 2 deletions packages/express/src/middleware-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,4 @@ export interface MiddlewareRegistry {
/**
* Base Context that provides APIs to register middleware
*/
export abstract class BaseMiddlewareRegistry extends MiddlewareMixin(Context)
implements MiddlewareRegistry {}
export abstract class BaseMiddlewareRegistry extends MiddlewareMixin(Context) {}
4 changes: 2 additions & 2 deletions packages/express/src/mixins/middleware.mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Constructor,
Context,
isBindingAddress,
MixinTarget,
Provider,
} from '@loopback/core';
import {
Expand Down Expand Up @@ -36,8 +37,7 @@ function extendsFrom(
return false;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function MiddlewareMixin<T extends Constructor<any>>(superClass: T) {
export function MiddlewareMixin<T extends MixinTarget<Context>>(superClass: T) {
if (!extendsFrom(superClass, Context)) {
throw new TypeError('The super class does not extend from Context');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Constructor} from '@loopback/context';
import {MixinTarget} from '@loopback/core';
import {Model, property} from '../../..';

/**
Expand All @@ -12,7 +12,7 @@ import {Model, property} from '../../..';
* @param superClass - Base Class
* @typeParam T - Model class
*/
export function AddCategoryPropertyMixin<T extends Constructor<Model>>(
export function AddCategoryPropertyMixin<T extends MixinTarget<Model>>(
superClass: T,
) {
class MixedModel extends superClass {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {Constructor} from '@loopback/context';
import {MixinTarget} from '@loopback/core';
import {CrudRepository, Model, Where} from '../../..';

/**
Expand All @@ -24,7 +24,7 @@ export interface FindByTitle<M extends Model> {
*/
export function FindByTitleRepositoryMixin<
M extends Model & {title: string},
R extends Constructor<CrudRepository<M>>
R extends MixinTarget<CrudRepository<M>>
>(superClass: R) {
class MixedRepository extends superClass implements FindByTitle<M> {
async findByTitle(title: string): Promise<M[]> {
Expand Down
Loading

0 comments on commit 4c03819

Please sign in to comment.