Skip to content
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

Use of @Action Decorator in Provider Class #4

Closed
wants to merge 6 commits into from
Closed

Use of @Action Decorator in Provider Class #4

wants to merge 6 commits into from

Conversation

bastikempken
Copy link

@bastikempken bastikempken commented Jul 10, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] Feature

What is the current behavior?

Action can be provided with higher order functions.

Issue Number: #2

What is the new behavior?

Action can be provided with higher order functions and with the use of the @Action decorator.

Does this PR introduce a breaking change?

[X] No

@bastikempken bastikempken changed the title WIP Feat/action provider Use of @Action Decorator in Provider Class Jul 10, 2020
@duffleit
Copy link
Collaborator

HI @bastikempken, thanks a lot for your contribution! Looks very good imho, why is the PR still in Draft-state? Just added a few minor comments.

src/lib/attach-action-provider.ts Outdated Show resolved Hide resolved
src/lib/attach-action-provider.ts Outdated Show resolved Hide resolved

const delegate = (stateContext: StateContext<any>, actionParam: any) =>
providerPropertyDescriptor.value.apply(provider.prototype, [stateContext, actionParam]);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to clarify what happens when attachActionProvider and attachAction provide handlers for the the same ActionType. For example an ActionA could be provided by attachAction and within the ActionProvider of attachActionProvider.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, do you have an idea how we can implement it? I think it is possible to check the information stored under META_KEY on the store class if an action is already present before adding. This is a more general inspection, not explicitly to avoid duplication caused by the redundant use of attachAction and attachActionProvider. There could also be a duplicate action which is also provided within a regular store class.
What is the error handling in ngxs/store when an action is provided twice in the same store class?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duffleit Friendly Reminder :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastikempken bastikempken marked this pull request as ready for review July 23, 2020 20:38
@seiyria
Copy link

seiyria commented Oct 19, 2022

Is there anything anyone can do to get this in motion again? This would be great to have.

@gtteamamxx
Copy link

Any update?

@bastikempken bastikempken closed this by deleting the head repository Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants