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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ An enum of all the possible saga injection behaviours
- `DAEMON` **[String][26]** Causes the saga to be started on component instantiation and never canceled
or started again.
- `ONCE_TILL_UNMOUNT` **[String][26]** Behaves like 'RESTART_ON_REMOUNT' but never runs it again.
- `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
Comment on lines +206 to +209
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!


[1]: #setup

Expand Down
7 changes: 6 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,19 @@ export function createInjectorsEnhancer(options: {
* @property {String} DAEMON Causes the saga to be started on component instantiation and never canceled
* or started again.
* @property {String} ONCE_TILL_UNMOUNT Behaves like 'RESTART_ON_REMOUNT' but never runs it again.
* @property {String} COUNTER 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
*
* @enum
* @public
*/
export enum SagaInjectionModes {
RESTART_ON_REMOUNT = "@@saga-injector/restart-on-remount",
DAEMON = "@@saga-injector/daemon",
ONCE_TILL_UNMOUNT = "@@saga-injector/once-till-unmount"
ONCE_TILL_UNMOUNT = "@@saga-injector/once-till-unmount",
COUNTER = "@@saga-injector/counter"
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
export const RESTART_ON_REMOUNT = '@@saga-injector/restart-on-remount';
export const DAEMON = '@@saga-injector/daemon';
export const ONCE_TILL_UNMOUNT = '@@saga-injector/once-till-unmount';
export const COUNTER = '@@saga-injector/counter';

export const COUNTER_PROP = `${COUNTER}:value`;

/**
* An enum of all the possible saga injection behaviours
Expand All @@ -10,6 +13,10 @@ export const ONCE_TILL_UNMOUNT = '@@saga-injector/once-till-unmount';
* @property {String} DAEMON Causes the saga to be started on component instantiation and never canceled
* or started again.
* @property {String} ONCE_TILL_UNMOUNT Behaves like 'RESTART_ON_REMOUNT' but never runs it again.
* @property {String} COUNTER 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
*
* @enum
* @public
Expand All @@ -18,4 +25,5 @@ export const SagaInjectionModes = {
RESTART_ON_REMOUNT,
DAEMON,
ONCE_TILL_UNMOUNT,
COUNTER,
};
66 changes: 57 additions & 9 deletions src/sagaInjectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,19 @@ import invariant from 'invariant';
import isEmpty from 'lodash/isEmpty';
import isFunction from 'lodash/isFunction';
import isString from 'lodash/isString';
import isNumber from 'lodash/isNumber';
import conformsTo from 'lodash/conformsTo';

import checkStore from './checkStore';
import { DAEMON, ONCE_TILL_UNMOUNT, RESTART_ON_REMOUNT } from './constants';
import {
DAEMON,
ONCE_TILL_UNMOUNT,
RESTART_ON_REMOUNT,
COUNTER,
COUNTER_PROP,
} from './constants';

const allowedModes = [RESTART_ON_REMOUNT, DAEMON, ONCE_TILL_UNMOUNT];
const allowedModes = [RESTART_ON_REMOUNT, DAEMON, ONCE_TILL_UNMOUNT, COUNTER];

const checkKey = key =>
invariant(
Expand All @@ -19,6 +26,7 @@ const checkDescriptor = descriptor => {
const shape = {
saga: isFunction,
mode: mode => isString(mode) && allowedModes.includes(mode),
[COUNTER_PROP]: isNumber,
};
invariant(
conformsTo(descriptor, shape),
Expand All @@ -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!

};
const { saga, mode } = newDescriptor;

Expand All @@ -50,16 +59,36 @@ export function injectSagaFactory(store, isValid) {
}
}

if (mode === COUNTER) {
// COUNTER must be added if saga is done
if (store.injectedSagas[key] === 'done') hasSaga = false;
let oldCounterValue = 0;
if (hasSaga) {
oldCounterValue = Math.max(
store.injectedSagas[key][COUNTER_PROP] || 0,
0,
);
}
newDescriptor[COUNTER_PROP] = oldCounterValue + 1;
}

if (
!hasSaga ||
(hasSaga && mode !== DAEMON && mode !== ONCE_TILL_UNMOUNT)
(hasSaga &&
mode !== DAEMON &&
mode !== ONCE_TILL_UNMOUNT &&
mode !== COUNTER)
) {
/* eslint-disable no-param-reassign */
store.injectedSagas[key] = {
...newDescriptor,
task: store.runSaga(saga),
};
/* eslint-enable no-param-reassign */
} else if (hasSaga && mode === COUNTER) {
// increment num of sagas that wants to be injected
/* eslint-disable no-param-reassign */
store.injectedSagas[key][COUNTER_PROP] = newDescriptor[COUNTER_PROP];
}
};
}
Expand All @@ -72,12 +101,31 @@ export function ejectSagaFactory(store, isValid) {

if (Reflect.has(store.injectedSagas, key)) {
const descriptor = store.injectedSagas[key];
if (descriptor.mode && descriptor.mode !== DAEMON) {
descriptor.task.cancel();
// Clean up in production; in development we need `descriptor.saga` for hot reloading
if (process.env.NODE_ENV === 'production') {
// Need some value to be able to detect `ONCE_TILL_UNMOUNT` sagas in `injectSaga`
store.injectedSagas[key] = 'done'; // eslint-disable-line no-param-reassign

if (descriptor.mode) {
if (descriptor.mode === COUNTER) {
descriptor[COUNTER_PROP] -= 1;

// don't cancel task if still not 0
if (descriptor[COUNTER_PROP] > 0) return;
}

if (descriptor.mode !== DAEMON) {
descriptor.task.cancel();
// Clean up in production; in development we need `descriptor.saga` for hot reloading
if (process.env.NODE_ENV === 'production') {
// Need some value to be able to detect `ONCE_TILL_UNMOUNT` sagas in `injectSaga`
store.injectedSagas[key] = 'done'; // eslint-disable-line no-param-reassign
}

// 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

descriptor.mode === COUNTER
) {
delete store.injectedSagas[key];
// only problem with this approach is that in development will have 'undefined' value
}
}
}
}
Expand Down
68 changes: 66 additions & 2 deletions src/tests/sagaInjectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ import getInjectors, {
injectSagaFactory,
ejectSagaFactory,
} from '../sagaInjectors';
import { DAEMON, ONCE_TILL_UNMOUNT, RESTART_ON_REMOUNT } from '../constants';
import {
COUNTER,
DAEMON,
ONCE_TILL_UNMOUNT,
RESTART_ON_REMOUNT,
COUNTER_PROP,
} from '../constants';
import { createInjectorsEnhancer } from '../createInjectorsEnhancer';

function configureStore() {
Expand Down Expand Up @@ -174,6 +180,9 @@ describe('injectors', () => {
expect(() =>
injectSaga('test', { saga: testSaga, mode: ONCE_TILL_UNMOUNT }),
).not.toThrow();
expect(() =>
injectSaga('test', { saga: testSaga, mode: COUNTER }),
).not.toThrow();
});

it('should not start daemon and once-till-unmount sagas if were started before', () => {
Expand All @@ -183,8 +192,11 @@ describe('injectors', () => {
injectSaga('test1', { saga: testSaga, mode: DAEMON });
injectSaga('test2', { saga: testSaga, mode: ONCE_TILL_UNMOUNT });
injectSaga('test2', { saga: testSaga, mode: ONCE_TILL_UNMOUNT });
injectSaga('test3', { saga: testSaga, mode: COUNTER });
injectSaga('test3', { saga: testSaga, mode: COUNTER });
injectSaga('test3', { saga: testSaga, mode: COUNTER });

expect(store.runSaga).toHaveBeenCalledTimes(2);
expect(store.runSaga).toHaveBeenCalledTimes(3);
});

it('should start any saga that was not started before', () => {
Expand Down Expand Up @@ -235,5 +247,57 @@ describe('injectors', () => {
injectSaga('test', { saga: testSaga, foo: 'bar' });
expect(store.injectedSagas.test.foo).toBe('bar');
});

it('should have correctly counter value in injectedSagas for COUNTER mode', () => {
function* testSaga1() {
yield put({ type: 'TEST', payload: 'yup' });
}

injectSaga('test', { saga: testSaga1, mode: COUNTER });
expect(store.injectedSagas.test[COUNTER_PROP]).toBe(1);

injectSaga('test', { saga: testSaga1, mode: COUNTER });
expect(store.injectedSagas.test[COUNTER_PROP]).toBe(2);

injectSaga('test', { saga: testSaga1, mode: COUNTER });
ejectSaga('test');
expect(store.injectedSagas.test[COUNTER_PROP]).toBe(2);

ejectSaga('test');
ejectSaga('test');
expect(store.injectedSagas.test).toBe(undefined);
});

it('should handle injection after ejecting all sagas', () => {
function* testSaga1() {
yield put({ type: 'TEST', payload: 'yup' });
}

injectSaga('test', { saga: testSaga1, mode: COUNTER });
injectSaga('test', { saga: testSaga1, mode: COUNTER });
ejectSaga('test');
ejectSaga('test');
expect(store.injectedSagas.test).toBe(undefined);

injectSaga('test', { saga: testSaga1, mode: COUNTER });
expect(store.injectedSagas.test[COUNTER_PROP]).toBe(1);
});

it('should not behave differently in production for COUNTER mode', () => {
process.env.NODE_ENV = 'production';
function* testSaga1() {
yield put({ type: 'TEST', payload: 'yup' });
}

injectSaga('test', { saga: testSaga1, mode: COUNTER });
injectSaga('test', { saga: testSaga1, mode: COUNTER });
ejectSaga('test');
ejectSaga('test');
expect(store.injectedSagas.test).toBe('done');

injectSaga('test', { saga: testSaga1, mode: COUNTER });
expect(store.injectedSagas.test[COUNTER_PROP]).toBe(1);
process.env.NODE_ENV = originalNodeEnv;
});
});
});