Skip to content

Added COUNTER mode for saga injectors #24

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

Merged
merged 2 commits into from
May 9, 2021

Conversation

goranurukalo
Copy link
Contributor

This PR does a few things:

Example

injectSaga({ key: "books", saga: booksSaga, mode: COUNTER })

I hope this helps somebody, cheers

Added 'COUNTER' constant for saga injectors mode property

This allows developers to use single saga many times in different components pages or similar systems

- Saga mounts only once, increments counter
- Saga unmounts only when all of same sagas are ejected
- Issues with takeLatest could appear
Copy link
Collaborator

@BenLorantfy BenLorantfy left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Looks good but I had a few suggestions

@@ -33,6 +41,7 @@ export function injectSagaFactory(store, isValid) {
const newDescriptor = {
...descriptor,
mode: descriptor.mode || DAEMON,
[COUNTER_PROP]: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just call this count ?

Suggested change
[COUNTER_PROP]: 0,
count: 0,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, its updated as mentioned!


// Clean up in development when mode is COUNTER
if (
process.env.NODE_ENV !== 'production' &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to check if not production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH i don't even remember, i will make example with issue and solution
to demonstrate what this actual does

I might figure out why i did that a long time ago
meanwhile i have removed it, because non of the tests fail

  • updated tests to support removal of this branch of code/logic

Comment on lines +206 to +209
- `COUNTER` **[String][26]** The saga will be mounted similar to 'RESTART_ON_REMOUNT', only difference is that
saga will be mounted only once on first inject, and ejected when all injectors are unmounted.
So this enables you to have multiple injectors with same saga and key, only one instance of saga will run
and enables you to have system that are more similar to widgets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- `COUNTER` **[String][26]** The saga will be mounted similar to 'RESTART_ON_REMOUNT', only difference is that
saga will be mounted only once on first inject, and ejected when all injectors are unmounted.
So this enables you to have multiple injectors with same saga and key, only one instance of saga will run
and enables you to have system that are more similar to widgets
- `COUNTER` **[String][26]** Similar to 'RESTART_ON_REMOUNT' except the
saga will be mounted only once on first inject and ejected when all injectors are unmounted.
This enables you to have multiple injectors with the same saga and key and only one instance of the saga will run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for help, i have updated as u suggested
now its much better!

- updated dynamic counter to count property of descriptor
- updated COUNTER mode description in comments and typescript types
- Removed delete part in development mode
- Added tests to cover removal of injected saga in dev env
@goranurukalo
Copy link
Contributor Author

I have created example where we demonstrate how counter works
check it out: https://github.com/goranurukalo/react-redux-saga-counter-mode-demo

Copy link
Collaborator

@BenLorantfy BenLorantfy left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 👍

@BenLorantfy BenLorantfy merged commit 441332e into react-boilerplate:dev May 9, 2021
@BenLorantfy BenLorantfy changed the title Implementing COUNTER mode for saga injectors Added COUNTER mode for saga injectors May 9, 2021
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.

2 participants