ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.01k stars 1.97k forks source link

feat(store): merge Action and TypedAction intefaces #4318

Closed timdeschryver closed 5 months ago

timdeschryver commented 5 months ago

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #4136 Closes #4183

What is the new behavior?

Does this PR introduce a breaking change?

[x] Yes
[ ] No
BREAKING CHANGE:

The Action and TypedAction interfaces are merged into one interface.

BEFORE:

There was a separation between the Action and TypedAction interfaces.

AFTER:

The Action interface accepts a generic type parameter that represents the payload type (defaults to string).
The TypedAction interface is removed.

Other information

netlify[bot] commented 5 months ago

Deploy Preview for ngrx-io canceled.

Name Link
Latest commit 625221587fb65c257f3aee73dd4a81e3dc89d26e
Latest deploy log https://app.netlify.com/sites/ngrx-io/deploys/663a78c6755fa30008445502
brandonroberts commented 5 months ago

I like this change. I also think its going to be a big breaking change for people who have reached into our src/models directory for that interface that will no longer be there. We've always declared that those are not public API and could be broken without warning, but just something we may want to note in the migration guide specifically.

trevorade commented 5 months ago

I like this change. I also think its going to be a big breaking change for people who have reached into our src/models directory for that interface that will no longer be there. We've always declared that those are not public API and could be broken without warning, but just something we may want to note in the migration guide specifically.

Could NgRx be structured in such a way such that only the index.ts files are accessible to remove the possibility of reaching into the implementation files?

brandonroberts commented 5 months ago

I like this change. I also think its going to be a big breaking change for people who have reached into our src/models directory for that interface that will no longer be there. We've always declared that those are not public API and could be broken without warning, but just something we may want to note in the migration guide specifically.

Could NgRx be structured in such a way such that only the index.ts files are accessible to remove the possibility of reaching into the implementation files?

We're already structured that way. We only expose code through the public API, but the types are included as deep imports. This is how the Angular library build process works with ng-packagr and not something custom we've set up.