-
-
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
docs(effects): update examples to standalone api #4522
base: main
Are you sure you want to change the base?
docs(effects): update examples to standalone api #4522
Conversation
✅ Deploy Preview for ngrx-io ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 for your work on this @AdrianRomanski
For next time, also try to add the issue number into the PR template (I've updated it now)
@@ -229,31 +242,6 @@ It's recommended to inject all dependencies as effect function arguments for eas | |||
|
|||
## Registering Root Effects | |||
|
|||
After you've written class-based or functional effects, you must register them so the effects start running. To register root-level effects, add the `EffectsModule.forRoot()` method with an array or sequence of effects classes and/or functional effects dictionaries to your `AppModule`. |
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.
Can you add a note about NgModules similar to https://github.com/sheikalthaf/platform/blob/6f926343e3f50e1d9a35c90b3c76c811d3fc9dbe/projects/ngrx.io/content/guide/router-store/index.md?plain=1#L30 please?
@@ -68,12 +75,18 @@ The component has multiple responsibilities: | |||
Effects handle external data and interactions, allowing your services to be less stateful and only perform tasks related to external interactions. Next, refactor the component to put the shared movie data in the `Store`. Effects handle the fetching of movie data. | |||
|
|||
<code-example header="movies-page.component.ts"> | |||
|
|||
import { Component, inject } from '@angular/core'; |
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.
If we're importing inject
here I think it's also good to update the snippet to use inject instead of using the constructor.
Do you also want to change this, or should we pick this up in a different PR?
<code-example header="movies-page.component.ts"> | ||
|
||
import { Component, inject } from '@angular/core'; |
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.
If we're importing inject
here I think it's also good to update the snippet to use inject instead of using the constructor.
Do you also want to change this, or should we pick this up in a different PR?
Note: the example of <code-example header="movies.service.ts">
also needs to be updated using injet
@@ -229,31 +242,6 @@ It's recommended to inject all dependencies as effect function arguments for eas | |||
|
|||
## Registering Root Effects |
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.
We can consolidate the Registering Root Effects
and Registering Feature Effects
into one, as it's the same for the standalone API.
Can you give this a try or do you'd like help with this?
E.g. the text "Registering effects can also be done using the standalone APIs..." also needs to be updated.
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.
@timdeschryver i did it, but not sure if its correct.
@@ -129,6 +129,7 @@ export function effectResubscriptionHandler>T extends Action<( | |||
); | |||
} | |||
|
|||
- im not sure what should be done with that one |
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 tink this page needs to undergo some more changes 😅
We could update this example to use bootstrapApplication
or leave it as is for later.
(but remove this comment please).
bootstrapApplication(
AppComponent,
{
providers: [
{
provide: EFFECTS_ERROR_HANDLER,
useValue: effectResubscriptionHandler,
},
{
provide: ErrorHandler,
useClass: CustomErrorHandler
}
],
}
)
…ctors, add missing imports
@ timdeschryver i pushed first changes :) Tommorrow i will do the remaining |
@AdrianRomanski are you willing to address the remaining feedback? |
@timdeschryver yes, i will sit to this straight after the office |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #4468
What is the new behavior?
Does this PR introduce a breaking change?
Other information