Skip to content

Commit

Permalink
fix(store): remove the recent @Select type safety check due to issu…
Browse files Browse the repository at this point in the history
…es with private/protected properties (ngxs#1623)

* test: expect errors when using private/protected with select decorator

* chore: rename state

* !fixup

* fix(store): remove implement SelectType<T> due to cannot assign @select to private/protected property

* chore: modifications method name

* chore: update CHANGELOG.md

* chore: update CHANGELOG.md

* chore: tweak CHANGELOG.md wording

* chore: tweak test name

Co-authored-by: Mark Whitfeld <[email protected]>
  • Loading branch information
splincode and markwhitfeld authored Jun 30, 2020
1 parent b82a912 commit cae998d
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ $ npm install @ngxs/store@dev
- Feature: Form Plugin - Add reset form action [#1604](https://github.com/ngxs/store/pull/1604)
- Performance: Logger Plugin - Plugin should lazy inject the store once [#1550](https://github.com/ngxs/store/pull/1550)
- Fix: `ofAction*` methods should prevent passing anything except of `ActionType` [#1616](https://github.com/ngxs/store/pull/1616)
- Fix: Remove the recent `@Select` type safety check due to issues with private/protected properties [#1623](https://github.com/ngxs/store/pull/1623)
- Fix: Actions are not canceled when any `Observable` returned by any handler is completed without emitting [#1615](https://github.com/ngxs/store/pull/1615)
- Fix: Router Plugin - Update state after route successfully activates [#1606](https://github.com/ngxs/store/pull/1606)
- Fix: HMR Plugin - Show error when use Angular Ivy with JIT mode [#1607](https://github.com/ngxs/store/pull/1607)
Expand Down
15 changes: 3 additions & 12 deletions packages/store/src/decorators/select/select.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
import {
ComponentClass,
createSelectObservable,
createSelectorFn,
PropertyType,
SelectType
} from './symbols';
import { createSelectObservable, createSelectorFn, PropertyType } from './symbols';

/**
* Decorator for selecting a slice of state from the store.
*/
export function Select<T>(rawSelector?: T, ...paths: string[]): SelectType<T> {
return function<
U extends ComponentClass<any> & Record<K, PropertyType<T>>,
K extends string
>(target: U, key: K): void {
export function Select<T>(rawSelector?: T, ...paths: string[]): PropertyDecorator {
return function(target, key): void {
const name: string = key.toString();
const selectorId = `__${name}__selector`;
const selector = createSelectorFn(name, rawSelector, paths);
Expand Down
12 changes: 0 additions & 12 deletions packages/store/src/decorators/select/symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,3 @@ export type PropertyType<T> = T extends StateToken<any>
: T extends (...args: any[]) => any
? Observable<ReturnType<T>>
: any;

export type ComponentClass<T> = {
[P in keyof T]: T[P];
};

export type SelectType<T> = <
U extends ComponentClass<any> & Record<K, PropertyType<T>>,
K extends string
>(
target: U,
key: K
) => void;
59 changes: 52 additions & 7 deletions packages/store/types/tests/selection.lint.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
/* tslint:disable:max-line-length */
/// <reference types="@types/jest" />
import { TestBed } from '@angular/core/testing';
import { NgxsModule, Select, Selector, State, Store } from '@ngxs/store';
import { NgxsModule, Select, Selector, State, StateToken, Store } from '@ngxs/store';
import { Observable } from 'rxjs';

import { assertType } from './utils/assert-type';
import { Component } from '@angular/core';
import {
createSelectObservable,
createSelectorFn,
PropertyType
} from '../../src/decorators/select/symbols';

describe('[TEST]: Action Types', () => {
let store: Store;
Expand All @@ -24,10 +30,10 @@ describe('[TEST]: Action Types', () => {
assertType(() => Selector([{ foo: 'bar' }])); // $ExpectType SelectorType<{ foo: string; }>
assertType(() => Selector({})); // $ExpectError

Select(); // $ExpectType SelectType<unknown>
assertType(() => Select({})); // $ExpectType SelectType<{}>
assertType(() => Select([])); // $ExpectType SelectType<never[]>
assertType(() => Select(Any, 'a', 'b', 'c')); // $ExpectType SelectType<typeof Any>
Select(); // $ExpectType PropertyDecorator
assertType(() => Select({})); // $ExpectType PropertyDecorator
assertType(() => Select([])); // $ExpectType PropertyDecorator
assertType(() => Select(Any, 'a', 'b', 'c')); // $ExpectType PropertyDecorator
assertType(() => Select(Any, ['a', 'b', 'c'])); // $ExpectError

class AppComponent {
Expand Down Expand Up @@ -93,9 +99,9 @@ describe('[TEST]: Action Types', () => {
class CheckSelectorComponent {
@Select() public A$: Observable<any>; // $ExpectType Observable<any>
@Select(TodoState) public B$: Observable<Any>; // $ExpectType Observable<Any>
@Select(TodoState.reverse) public C$: Observable<Any>; // $ExpectError
@Select(TodoState.reverse) public C$: Observable<Any>; // $ExpectType Observable<Any>
@Select(TodoState.reverse) public C1$: Observable<string[]>; // $ExpectType Observable<string[]>
@Select(TodoState.reverse) public D$: number | object; // $ExpectError
@Select(TodoState.reverse) public D$: number | object; // $ExpectType number | object
@Select(TodoState.reverse) public D1$: Observable<string[]>; // $ExpectType Observable<string[]>
}

Expand Down Expand Up @@ -138,4 +144,43 @@ describe('[TEST]: Action Types', () => {
}
}
});

it('should support protected and private methods (https://github.com/ngxs/store/issues/1532)', () => {
const TODOS_TOKEN = new StateToken<string[]>('todos');

@State({ name: TODOS_TOKEN })
class TodosState {
@Selector([TODOS_TOKEN]) // $ExpectType (state: string[]) => string[]
public static publicState(state: string[]) {
return state;
}

@Selector([TODOS_TOKEN]) // $ExpectType (state: string[]) => string[]
protected static protectedState(state: string[]) {
return state;
}

@Selector([TODOS_TOKEN]) // $ExpectType (state: string[]) => string[]
private static privateState(state: string[]) {
return state;
}
}

@Component({ selector: 'app' })
class AppComponent {
// $ExpectType Observable<any>
@Select(TODOS_TOKEN) public readonly publicTodos: Observable<any>;

// $ExpectType Observable<any>
@Select(TODOS_TOKEN) protected readonly protectedTodos: Observable<any>;

// $ExpectType Observable<any>
@Select(TODOS_TOKEN) private readonly privateTodos: Observable<any>;
}

TestBed.configureTestingModule({
declarations: [AppComponent],
imports: [NgxsModule.forRoot([TodosState])]
});
});
});
10 changes: 5 additions & 5 deletions packages/store/types/tests/state-token.lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,16 @@ describe('[TEST]: StateToken', () => {
class AppComponent {
@Select() appV1$: string; // $ExpectType string
@Select() appV2$: string; // $ExpectType string
@Select((state: string) => state) appV3$: string; // $ExpectError
@Select((state: string) => state) appV3$: string; // $ExpectType string
@Select((state: string) => state) appV3_1$: Observable<string>; // $ExpectType Observable<string>

// Argument of type is not assignable to parameter of type Observable<{ foo: boolean }>
@Select(FOO_TOKEN) appV4$: string; // $ExpectError
@Select(FOO_TOKEN) appV5$: Observable<string>; // $ExpectError
@Select(FOO_TOKEN) appV4$: string; // $ExpectType string
@Select(FOO_TOKEN) appV5$: Observable<string>; // $ExpectType Observable<string>
@Select(FOO_TOKEN) appV6$: Observable<MyModel>; // $ExpectType Observable<MyModel>

@Select(FooState.bar) bar$: string; // $ExpectError
@Select(FooState.bar) bar1$: Observable<string>; // $ExpectError
@Select(FooState.bar) bar$: string; // $ExpectType string
@Select(FooState.bar) bar1$: Observable<string>; // $ExpectType Observable<string>
@Select(FooState.bar) bar2$: Observable<boolean>; // $ExpectType Observable<boolean>

constructor(public store: Store) {}
Expand Down

0 comments on commit cae998d

Please sign in to comment.