Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: enable type checks inside mixin classes #5394

Merged
merged 1 commit into from
May 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>
bajtos marked this conversation as resolved.
Show resolved Hide resolved
>(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 ?? {},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem was hidden by any before.

);
}

/**
* 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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem was hidden by any before.

}

/**
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
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ export * from './extension-point';
export * from './keys';
export * from './lifecycle';
export * from './lifecycle-registry';
export * from './mixin-target';
export * from './server';
export * from './service';
70 changes: 70 additions & 0 deletions packages/core/src/mixin-target.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];
}
>;
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