-
-
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
refactor: consistent naming of injection tokens #2737
refactor: consistent naming of injection tokens #2737
Conversation
Preview docs changes for f4a6353 at https://previews.ngrx.io/pr2737-f4a6353c/ |
aa5116a
to
f4a6353
Compare
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 👍
@brandonroberts > ng run data:test
Nx could not find process's output. Run the command without --parallel. I restarted the workflow, hope it will fix it. |
@@ -32,7 +32,9 @@ export interface SelectConfig { | |||
debounce?: boolean; | |||
} | |||
|
|||
export const initialStateToken = new InjectionToken('ComponentStore InitState'); | |||
export const INITIAL_STATE_TOKEN = new InjectionToken( |
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 forgot this PR.
I was thinking if this could be seen as a breaking change 🤔
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.
That token is part of the public API, so I would say yes
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'll tag it for v11 :)
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.
ComponentStore's token was not supposed to be exposed 😐 It's necessary for ViewEngine backwards-compatibility.
I'm fine with leaving it for v11, at the same time I'm suspecting that our next release would be v11 🙂
…2737) BREAKING CHANGE: The initial state Injection Token for `@ngrx/component-store` has been renamed BEFORE: Injection Token is `initialStateToken` AFTER: Injection Token is `INITIAL_STATE_TOKEN`
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?
Name/description of injection tokens is inconsistent.
@ngrx/component-store
@ngrx/data
@ngrx/effects
etc.
What is the new behavior?
Name/description of injection tokens is consistent.
Format:
@ngrx/module-name Title Cased Description
Does this PR introduce a breaking change?
Other information