GuillaumeCisco / redux-sagas-injector

Helper for loading sagas asynchronously using redux
MIT License
69 stars 7 forks source link

Strange interaction with Storybook and remounting components #17

Closed thehig closed 2 months ago

thehig commented 2 months ago

Strange interaction with Storybook

Scenario

The above works perfectly when I refresh the page, but has some weird behavior if I try and 'remount' the component.

Remounting the component in this context means that it will generate a new store, new reducers etc

However

It seems to me that the sagaMiddleware is a singleton, which is resulting in some strange behavior.

When I remount this component, everything seems to be working properly, but saga events are being duplicated.

eg:

  • My automation will click the 'LOGIN' button which is a takeEvery() saga trigger.
  • This dispatches a FETCH_API action
  • In practise however, this dispatches N FETCH_API actions, one for each time the component was mounted
  • This then means there are N duplicates of the FETCH_SUCCESS and all of the subsequent loader actions

I found that if I added the following, I got different results:

// In another file
export function* sagas() {
  yield all([
    call(ApiSaga),
    call(UISaga),
    call(AuthSaga),
    call(TenantSaga),
    call(ServerlistSaga),
  ]);
}

// In my mock-store creator after `createStore()`
const sagaMiddleware = actualMiddlewares[1];

if (sagaMiddleware.running) {
  console.log('Preventing saga restart');
} else {
  console.log('Starting sagas');
  sagaMiddleware.run(sagas);
  sagaMiddleware.running = true;
}

With this change preventing the middleware from starting multiple times (even though they are separate instances as far as I can control them) I see the following behavior:

eg:

My automation will click the 'LOGIN' button which is a takeEvery() saga trigger. This saga trigger dispatches a FETCH_API action This is picked up by the already running saga and results in FETCH_SUCCESS etc However my component doesn't update, which tells me that the store context in which the saga is operating is disconnected from the one my component/new store had intended

I don't know how to proceed here. I've tried everything I can think of to destroy/recreate/reset/clear the saga middleware in between component mounts, but I just can't seem to find a way that will satisfy my testing conditions.

Can someone suggest something?

GuillaumeCisco commented 2 months ago

Hello @thehig

Thank you very much for your detailed message. I really appreciate.

Unfortunately I did not work on this project for 4 years now :/ Back in the days, I struggled a lot to make this library alive. Middlewares and hot injection in redux are not so easy to tame.

You should look at the source code of this lib, it is less than 100 lines, very straight forward, but requires to be focus on what you will read.

Maybe you will find in a few minutes the eureka moment.

I am sorry I could not help more.

thehig commented 2 months ago

So I copied the code into my codebase directly, refactored it to Typescript to help me understand it, but unfortunately I was unable to resolve the issue directly.

As an indirect fix however, I was able to switch to the out-of-the-box redux-saga middleware which worked enough to pass my tests reliably.

Thanks for the input and the quick reply

thehig commented 2 months ago
/**
 * Created by guillaume on 1/17/17.
 * Copied from https://github.com/GuillaumeCisco/redux-sagas-injector/blob/master/src/redux-sagas-injector.js by @thehig on 4/18/24
 */

import {
  createInjectStore,
  type InjectedStore,
  RecursiveReducerMap,
  deleteStore as deleteInjectedStore,
} from './redux-reducers-injector';
import createSagaMiddleware, { type Saga } from 'redux-saga';
import { take, fork, cancel } from 'redux-saga/effects';

export {
  injectReducer,
  reloadReducer,
  injectReducerBulk,
} from './redux-reducers-injector';

export const CANCEL_SAGAS_HMR = 'CANCEL_SAGAS_HMR';
const DEBUG = false;

export type InjectedSagaStore = InjectedStore & {
  injectedSagas: string[];
};

let original_store: InjectedSagaStore | undefined;

/**
 *
 * store.injectedSagas [] is used to keep track of sagas that have been injected.
 *                        it is managed by the injectSaga and injectSagaBulk functions.
 */

export const sagaMiddleware = createSagaMiddleware();

/**
 * In development env, creates a saga that can be cancelled by HMR (Hot Module Replacement)
 * In production, returns the original saga. Does nothing.
 */
function createAbortableSaga(key: string, saga: Saga) {
  DEBUG &&
    console.log('[redux-sagas-injector][createAbortableSaga]', key, saga);

  if (process.env.NODE_ENV === 'development') {
    return function* main() {
      const sagaTask: ReturnType<typeof saga> = yield fork(saga);
      const { payload } = yield take(CANCEL_SAGAS_HMR);

      if (payload === key) {
        // @ts-expect-error I can't figure out how to type this correctly
        yield cancel(sagaTask);
      }
    };
  } else {
    return saga;
  }
}

/**
 * Singleton object to start/cancel sagas by key.
 *
 * !Uses the singleton sagaMiddleware and original_store objects.
 */
export const SagaManager = {
  startSaga(key: string, saga: Saga): void {
    DEBUG && console.log('[redux-sagas-injector][startSaga]', key, saga);

    sagaMiddleware.run(createAbortableSaga(key, saga) as Saga);
  },

  cancelSaga(key: string, store = original_store): void {
    DEBUG && console.log('[redux-sagas-injector][cancelSaga]', key, store);

    store?.dispatch({
      type: CANCEL_SAGAS_HMR,
      payload: key,
    });
  },
};

export function reloadSaga(
  key: string,
  saga: Saga,
  store = original_store
): void {
  DEBUG && console.log('[redux-sagas-injector][reloadSaga]', key, saga, store);

  SagaManager.cancelSaga(key, store);
  SagaManager.startSaga(key, saga);
}

export function injectSaga(
  key: string,
  saga: Saga,
  force = false,
  store = original_store
): void {
  DEBUG &&
    console.log('[redux-sagas-injector][injectSaga]', key, saga, force, store);

  // If already set, do nothing, except force is specified
  const exists = store?.injectedSagas.includes(key);
  if (!exists || force) {
    if (!exists && store) {
      store.injectedSagas = [...store.injectedSagas, key];
    }
    if (force) {
      SagaManager.cancelSaga(key, store);
    }
    SagaManager.startSaga(key, saga);
  }
}

export function injectSagaBulk(
  sagas: Array<{ key: string; saga: Saga }>,
  force = false,
  store = original_store
): void {
  DEBUG &&
    console.log('[redux-sagas-injector][injectSagaBulk]', sagas, force, store);

  sagas.forEach((x) => {
    // If already set, do nothing, except force is specified
    const exists = store?.injectedSagas.includes(x.key);
    if (!exists || force) {
      if (!exists && store) {
        store.injectedSagas = [...store.injectedSagas, x.key];
      }
      if (force) {
        SagaManager.cancelSaga(x.key, store);
      }
      SagaManager.startSaga(x.key, x.saga);
    }
  });
}

export function createInjectSagasStore(
  rootSaga: Record<string, Saga>,
  initialReducers: RecursiveReducerMap,
  ...args: unknown[]
): InjectedSagaStore {
  DEBUG &&
    console.log(
      '[redux-sagas-injector][createInjectSagasStore]',
      rootSaga,
      initialReducers,
      args
    );

  original_store = {
    ...createInjectStore(initialReducers, ...args),
    injectedSagas: [],
  };

  injectSaga(
    Object.keys(rootSaga)[0],
    rootSaga[Object.keys(rootSaga)[0]],
    false,
    original_store
  );

  return original_store;
}

export function deleteStore(): void {
  DEBUG && console.log('[redux-sagas-injector][deleteStore]');

  deleteInjectedStore();
  original_store = undefined;
}

export default createInjectSagasStore;
// https://raw.githubusercontent.com/GuillaumeCisco/redux-reducers-injector/master/src/ReduxInjector.js

import {
  createStore,
  combineReducers as originalCombineReducers,
  type Reducer,
  type Store,
} from 'redux';

import set from 'lodash/set';
import has from 'lodash/has';

const DEBUG = false;

/**
 * Recursive map of reducers
 */
export interface RecursiveReducerMap {
  [key: string]: Reducer | RecursiveReducerMap;
}
export type InjectedStore = Store & { injectedReducers: RecursiveReducerMap };

/**
 * Redux store with additional `injectedReducers` property
 */
let original_store: InjectedStore | undefined;

/**
 * The original `combineReducers` imported from `redux` directly
 *
 * Can be overridden inside `createInjectStore()`
 */
let combineReducers = originalCombineReducers;

/**
 * Recursively combine reducers into a single reducer using `combineReducers()`
 */
export function combineReducersRecursively(
  reducers: Reducer | RecursiveReducerMap
): Reducer {
  DEBUG &&
    console.log(
      '[redux-reducers-injector][combineReducersRecursively]',
      reducers
    );

  // If this is a leaf or already combined.
  if (typeof reducers === 'function') {
    return reducers;
  }

  // If this is an object of functions, combine reducers.
  if (typeof reducers === 'object') {
    const combinedReducers: Record<string, Reducer> = {};

    const keys = Object.keys(reducers);
    for (let i = 0; i < keys.length; i++) {
      const key = keys[i];
      combinedReducers[key] = combineReducersRecursively(reducers[key]);
    }

    return combineReducers(combinedReducers);
  }

  // If we get here we have an invalid item in the reducer path.
  throw new Error('Invalid item in reducer tree');
}

/**
 * Create a new Redux store with the ability to inject reducers
 *
 * @param initialReducers Initial reducers to inject
 * @param args Additional arguments to pass to `createStore()`
 */
export function createInjectStore(
  initialReducers: RecursiveReducerMap,
  ...args: unknown[]
): InjectedStore {
  DEBUG &&
    console.log(
      '[redux-reducers-injector][createInjectStore]',
      initialReducers,
      args
    );

  // If last item is an object, it is overrides.
  if (typeof args[args.length - 1] === 'object') {
    const overrides = args.pop() as {
      combineReducers?: typeof originalCombineReducers;
    };
    // Allow overriding the combineReducers function such as with redux-immutable.
    if (
      Object.prototype.hasOwnProperty.call(overrides, 'combineReducers') &&
      typeof overrides.combineReducers === 'function'
    ) {
      combineReducers = overrides.combineReducers;
    }
  }

  // TODO: Switch to createStore() from redux-toolkit
  original_store = createStore(
    combineReducersRecursively(initialReducers),
    // @ts-expect-error Shhhhh
    ...args
  );

  // Store the map of injected reducers as a kind of index
  original_store.injectedReducers = initialReducers;

  return original_store;
}

/**
 * Replace a reducer in the store with a new reducer
 *
 * @param key Key of the reducer to replace
 * @param reducer New reducer
 * @param store Redux store to replace reducer in (default: `original_store`)
 */
export function reloadReducer(
  key: string,
  reducer: Reducer,
  store = original_store
): void {
  DEBUG &&
    console.log(
      '[redux-reducers-injector][reloadReducer]',
      key,
      reducer,
      store
    );

  store?.replaceReducer(
    combineReducersRecursively({ ...store?.injectedReducers, [key]: reducer })
  );
}

/**
 * Inject a new reducer into the store
 * If the key already exists, it will not be replaced unless `force` is `true`
 *
 * @param key Key of the reducer to inject
 * @param reducer Reducer to inject
 * @param force Replace the reducer if it already exists
 * @param store Redux store to inject reducer into (default: `original_store`)
 */
export function injectReducer(
  key: string,
  reducer: Reducer,
  force = false,
  store = original_store
): void {
  if (!store) return;
  DEBUG &&
    console.log(
      '[redux-reducers-injector][injectReducer]',
      key,
      reducer,
      force,
      store
    );

  // If already set, do nothing.
  if (!has(store.injectedReducers, key) || force) {
    set(store.injectedReducers, key, reducer);
    store.replaceReducer(combineReducersRecursively(store.injectedReducers));
  }
}

/**
 * Inject multiple reducers into the store
 * If a key already exists, it will not be replaced unless `force` is `true`
 *
 * @param reducers Array of `{ key, reducer }` to inject
 * @param force Replace the reducer if it already exists
 * @param store Redux store to inject reducer into (default: `original_store`)
 */
export function injectReducerBulk(
  reducers: Array<{ key: string; reducer: Reducer }>,
  force = false,
  store = original_store
): void {
  if (!store) return;
  DEBUG &&
    console.log(
      '[redux-reducers-injector][injectReducer]',
      reducers,
      force,
      store
    );

  let update = false;
  reducers.forEach((x) => {
    // If already set, do nothing.
    if (!has(store.injectedReducers, x.key) || force) {
      set(store.injectedReducers, x.key, x.reducer);
      update = true;
    }
  });

  if (update) {
    store.replaceReducer(combineReducersRecursively(store.injectedReducers));
  }
}

export function deleteStore(): void {
  DEBUG && console.log('[redux-reducers-injector][deleteStore]');

  original_store = undefined;
}
GuillaumeCisco commented 2 months ago

Hello @thehig ,

Thank you for your insight on this. I am glad you were able to find a code for your needs.

Regarding typescript, I don't really understand why you needed it. From my opinion (and I hope people are able to respect it), typescript is the worst thing that happened to javascript. I will never put this broken language in one of my own projects.

thehig commented 2 months ago

Regarding typescript, I don't really understand why you needed it.

Adding Typescript on top was just an exercise to help me 'grok' the code. It doesn't/didn't add anything to the functionality.

From my opinion (and I hope people are able to respect it), typescript is the worst thing that happened to javascript. I will never put this broken language in one of my own projects.

I disagree, but not without understanding your position. <3