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(store): deprecate @Select #2135

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Conversation

arturovt
Copy link
Member

No description provided.

Copy link

bundlemon bot commented Mar 21, 2024

BundleMon (Integration Projects)

Unchanged files (2)
Status Path Size Limits
Main bundles(Gzip)
hello-world-ng17/dist-integration/main.(hash)
.js
68.43KB +1%
Main bundles(Gzip)
hello-world-ng16/dist-integration/main.(hash)
.js
66.93KB +1%

No change in files bundle size

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Copy link

codeclimate bot commented Mar 21, 2024

Code Climate has analyzed commit 7711669 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 94.7% (-0.7% change).

View more on Code Climate.

Copy link
Member

Choose a reason for hiding this comment

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

This file should be in the recipes folder. The advanced folder is disappearing soon.

@@ -2,6 +2,9 @@ import { createSelectObservable, createSelectorFn, PropertyType } from './symbol

/**
* Decorator for selecting a slice of state from the store.
*
* @deprecated
* Read the deprecation notice at this link: https://ngxs.io/advanced/select-decorator-deprecation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Read the deprecation notice at this link: https://ngxs.io/advanced/select-decorator-deprecation.
* Read the deprecation notice at this link: https://ngxs.io/recipes/select-decorator-deprecation.

@@ -68,250 +66,6 @@ describe('Select', () => {

const states = [MySubState, MySubSubState, MyState];

it('should throw an exception when the user has forgotten to import the NGXS module', () => {
Copy link
Member

Choose a reason for hiding this comment

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

We should keep these tests for @Select until the decorator is actually removed. We might need to add a linting hint to ignore the deprecation notice here.

@@ -40,8 +40,7 @@ describe('[TEST]: StateToken', () => {
template: '{{ myState$ | async | json }}'
})
class MyComponent {
@Select(TODO_LIST_TOKEN)
myState$: Observable<string[]>;
myState$: Observable<string[]> = inject(Store).select(TODO_LIST_TOKEN);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is cleaner?
Use the store reference that we already have.

Suggested change
myState$: Observable<string[]> = inject(Store).select(TODO_LIST_TOKEN);
myState$ = this.storeApp.select(TODO_LIST_TOKEN);

Selector(); // $ExpectType SelectorType<unknown>
assertType(() => Selector([{ foo: 'bar' }])); // $ExpectError
assertType(() => Selector({})); // $ExpectError

Select(); // $ExpectType PropertyDecorator
Copy link
Member

Choose a reason for hiding this comment

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

We should keep these tests until we actually remove the decorator.


type Any = number | string | boolean | object;

class CheckSelectorComponent {
Copy link
Member

Choose a reason for hiding this comment

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

As above, and keep this too.

@@ -158,22 +122,5 @@ describe('[TEST]: Action Types', () => {
return state;
}
}

@Component({ selector: 'app' })
class AppComponent {
Copy link
Member

Choose a reason for hiding this comment

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

As above, and keep this too.

@@ -133,63 +129,4 @@ describe('[TEST]: StateToken', () => {

NgxsModule.forRoot([TodoListState]);
});

it('should be correct type checking in select', () => {
Copy link
Member

Choose a reason for hiding this comment

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

As above, and keep this too.

Copy link
Member

@markwhitfeld markwhitfeld left a comment

Choose a reason for hiding this comment

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

Almost there.

Copy link
Member

Choose a reason for hiding this comment

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

Need to link this file in the SUMMARY.md document under the recipes section.

@markwhitfeld markwhitfeld merged commit 0947c22 into master Mar 21, 2024
5 of 7 checks passed
@markwhitfeld markwhitfeld deleted the ref/deprecate-select branch March 21, 2024 17:03
@markwhitfeld markwhitfeld added this to the v.18.0.0 milestone Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants