Don't handle nil actions in navigation middleware

[serhiipalash]

The use case is when you have custom action creator attached to router with "getCustomActionCreators" config property, but it doesn't alway return an action.

Example is "handleLink" function which takes url argument, validates it and, if it is a valid deep link, updates the navigation state, but, if url starts with https://, it opens it in WebBrowser and doesn't return any action. There are other use cases (we have many custom action creators), but this is the main.

To fix it today, we need to always return empty action from each custom action creator.

P.S. react-navigation routers already skip nil actions and don't call getStateForAction. So, I guess this package should follow general practice too.

[Ashoat]

The purpose of the middleware is to trigger the event listeners in the same way that the stock createAppContainer does. That code triggers on null actions, so to be consistent we also trigger on null actions. The router code you reference is downstream of the action dispatch code that handles triggering event listeners, and is thus irrelevant to the middleware code here.

[serhiipalash]

Ok. Than maybe we should add check for nil action here https://github.com/serhiipalash/redux-helpers/blob/daa5b54a17861416fbd1d84244dfa36911f653a9/src/middleware.js#L64 ?

function triggerAllSubscribers(key: string, payload: NavigationEventPayload) {
  const trigger =
    () => getReduxSubscribers(key).forEach(subscriber => subscriber(payload));
  if (
    !payload.action.hasOwnProperty('type') ||
    !payload.action.type.startsWith("Navigation") ||
    payload.state === payload.lastState
  ) {
    trigger();
    return;
  }

and change it to

function triggerAllSubscribers(key: string, payload: NavigationEventPayload) {
  const trigger =
    () => getReduxSubscribers(key).forEach(subscriber => subscriber(payload));
  if (
    !payload.action ||
    !payload.action.hasOwnProperty('type') ||
    !payload.action.type.startsWith("Navigation") ||
    payload.state === payload.lastState
  ) {
    trigger();
    return;
  }

Currently the middleware crashes the app on the line !payload.action.hasOwnProperty('type') || if action is null. I am not sure what is the behaviour of createAppContainer, but does it crash the app too on nil actions?

[Ashoat]

Okay, that makes more sense to me

[serhiipalash]

Okay, that makes more sense to me

@Ashoat please review this pr https://github.com/react-navigation/redux-helpers/pull/95