-
Notifications
You must be signed in to change notification settings - Fork 34
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(ngrx-toolkit): Creates provideDevtoolsConfig #151
feat(ngrx-toolkit): Creates provideDevtoolsConfig #151
Conversation
…toolsConfig to configure redux devtools
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 the PR. I've left a few notes.
/** | ||
* Provides the configuration options for connecting to the Redux DevTools Extension. | ||
*/ | ||
export function provideDevtoolsConfig( |
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 think we should only provide those options which we are really supporting. So at the moment that would only be the name.
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.
Do we have to manually handle maxAge? I could see that as a useful one as well.
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.
At the moment, we can't really support anything of the options of the original. It would be great if we get them, but not in this PR.
docs/docs/provide-devtools-config.md
Outdated
@@ -0,0 +1,64 @@ | |||
--- |
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.
this will be part of with-redux.md
. You can add an additional chapter there, where you mention the possibility. Please use bootstrapApplication
in the example instead of an NgModule
.
libs/ngrx-toolkit/src/lib/devtools/tests/provide-devtools-config.spec.ts
Show resolved
Hide resolved
@rainerhahnekamp - Thank you for the feedback. I've pushed a new commit to address your comments. |
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 see that you would like to support the options of the Devtools.
What about the following suggestion. We do this PR just with name, and then we can check in a separate one, would options are supported automatically by the DevTools and for which options we need to provide additional implementation?
docs/docs/with-redux.md
Outdated
providers: [ | ||
provideDevtoolsConfig({ | ||
name: 'MyApp', | ||
maxAge: 50, |
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.
Needs to be removed here as well.
docs/docs/with-redux.md
Outdated
|
||
### Additional Information | ||
|
||
For more details on the available options and their usage, refer to the [Redux DevTools Extension documentation](https://github.com/zalmoxisus/redux-devtools-extension). |
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 should use the right link: https://github.com/reduxjs/redux-devtools
docs/docs/with-redux.md
Outdated
The `provideDevtoolsConfig` function accepts an object with the following properties: | ||
|
||
- `name` (string): Optional name for the DevTools instance. The default is "NgRx SignalStore". | ||
- `maxAge` (number): Maximum number of actions to keep in the history. The default is 50. |
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.
No maxAge
. We don't support it. That would be a separate issue.
docs/docs/with-redux.md
Outdated
await bootstrapApplication(AppComponent, appConfig); | ||
``` | ||
|
||
### Options |
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.
Since we have only name
, I guess we can remove the options.
docs/docs/with-redux.md
Outdated
|
||
The `provideDevtoolsConfig` function allows you to configure the Redux DevTools integration for your NgRx SignalStore. This function is essential for setting up the DevTools with custom options. The function only needs to be called once in your appConfig or AppModule. | ||
|
||
### Usage |
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.
No need for a separate header?
/** | ||
* Provides the configuration options for connecting to the Redux DevTools Extension. | ||
*/ | ||
export function provideDevtoolsConfig( |
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.
At the moment, we can't really support anything of the options of the original. It would be great if we get them, but not in this PR.
* to the Redux DevTools Extension's connect function. It includes options | ||
* for enabling features, serialization, tracing, action creators, and other | ||
* settings that control the behavior and appearance of the DevTools. | ||
* See {@link https://github.com/reduxjs/redux-devtools/blob/main/extension/docs/API/Arguments.md the Redux DevTools API documentation}. |
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 don't support them yet.
@rainerhahnekamp - Thanks for the great suggestions again. I have updated the PR to align with your comments. |
1000 thanks @mikerentmeister. Expect a new release of the toolkit with your feature soon! |
Creates a provideDevtoolsConfig function to allow the configuration of the Redux Devtools Extension.
fixes #149