-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(component): make LetDirective and PushPipe standalone #3826
feat(component): make LetDirective and PushPipe standalone #3826
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
@timdeschryver , @markostanimirovic Could you maybe have a look? Btw, do you have any idea why the build is failing? |
@stefanoslig this issue was already to assigned to be worked on, ping them in the original issue to see if they still want to work on it first before proceeding here |
I closed this since @adyngom is going to work on this |
@brandonroberts could you maybe help me with the failed pipeline? |
…xamples for the deprecated code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @stefanoslig! few suggestions 👇
…based on the feedback in the PR
.yarn/install-state.gz
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this file is pushed by mistake 👀
@@ -154,6 +154,129 @@ import { createMockStore } from '@ngrx/store/testing'; | |||
const mockStore = createMockStore(); | |||
``` | |||
|
|||
#### LetModule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecations need to be added as a separate section. Move added lines to the bottom of this file and add the following headings:
## Deprecations
### @ngrx/component
#### LetModule
....
#### PushModule
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, yeah, you're right!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤝
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
With this PR we make the LetDirective and PushPipe standalone. The LetModule and PushModule have been marked as deprecated and they can still be used.
Ideally, I would like to rename the directive from
LetDirective
=>NgRxLet
and the pipe fromPushPipe
=>NgRxPush
but this would introduce a lot of changes. Maybe something for the future?Another thing we can consider for the future is a migration schematic.
What is the current behavior?
Closes #
#3804
What is the new behavior?
Does this PR introduce a breaking change?
Other information