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

ngrxPush not working with strictTemplate checks #3114

Closed
ocsurfnut opened this issue Aug 13, 2021 · 1 comment · Fixed by #3127
Closed

ngrxPush not working with strictTemplate checks #3114

ocsurfnut opened this issue Aug 13, 2021 · 1 comment · Fixed by #3127

Comments

@ocsurfnut
Copy link

Minimal reproduction of the bug/regression with instructions:

Use the | ngrxPush pipe with an observable typed differently than the @Input().

Expected behavior:

Error Type 'X' is not assignable to type 'Y'.

Actual behavior:

No type error in IDE or from ng build.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

Versions 11 and 12, not OS-specific.

Other information:

Copying the ngrxPush source code locally and experimenting, I've isolated it to this line. If export class PushPipe<S> ... is changed to export class PushPipe<S = any> ..., the type checking starts working.

Working from there, I confirmed that any pipe class with a generic argument is skipped in strict template checking. I created a Stackblitz to demonstrate the base issue, which seems specific to Angular, not NgRx. However, I couldn't find any related issues with Angular or otherwise.

Without understanding the need for this generic parameter, I'm not sure if this change is desirable. I noticed the Angular async pipe simply types the internal value as any, and does not use a generic on the class.

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@markostanimirovic
Copy link
Member

Generic parameter here seems like a surplus to me too.

Reproduction with ngrxPush

brandonroberts pushed a commit that referenced this issue Nov 2, 2021
Closes #3114 

BREAKING CHANGES:

PushPipe no longer has a class-level generic type parameter.

BEFORE:

Use of PushPipe outside of component templates required a generic

AFTER:

Use of PushPipe outside of component templates no longer requires a generic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants