Update flags

[montogeek]

Describe the bug I am unable to update flags using UPDATE_FLAGS constant from @flopflip/react-redux when using memory-adapter and giving initial state using createFlipFlopReducer.

You can view a demo of this error here: https://codesandbox.io/s/q8n8njyyow?fontsize=14, please open Page.js file.

To Reproduce Steps to reproduce the behavior:

1. Given

   dispatch({
     type: UPDATE_FLAGS,
     payload: {
       flags: {
         socialLogin: {
           facebook: true,
           google: true
         }
       }
     }
   });

2. Then state is:

   {
   '@flopflip': {
   flags: {
     socialLogin: {
       facebook: false,
       google: false
     },
     flags: {
       socialLogin: {
         facebook: true,
         google: true
       }
     }
   },
   status: {
     status: {
       isReady: false
     }
   }
   }
   }

   The reason is that the state doesn't have a flag property: But the reducer expects it.

It would work if the reducer is initiated this way:

export default createFlopflipReducer({ flags: features });

since FlipFlop state will now have a flags key in the flags object.

But it will break selectFeatureFlag and similar because they expect flags to be a direct child of reducer state.

Expected behavior I expect it to replace or merge the flags object instead of appending it.

Screenshots Demo at: https://codesandbox.io/s/q8n8njyyow?fontsize=14

Additional context

• I found that memory-adapter has an action creator (https://github.com/tdeekens/flopflip/blob/master/packages/react-redux/modules/ducks/flags/flags.ts#L38) that is unfortunately not exported by the module (https://github.com/tdeekens/flopflip/blob/master/packages/react-redux/modules/index.ts) although docs say it does (https://github.com/tdeekens/flopflip/blob/master/readme.md#L262)

• I haven't search how the status isReady boolean works, in my case it is always false.

[tdeekens]

Thanks a lot for the very detailed bug description. It's truely the most helpful I've received! I will try to have a look at this tomorrow and hopefully get a fix out or at least comment with something more meaninful than this here :)

[montogeek]

@tdeekens Not problem, as a fellow OSS maintainer, I feel the pain of poorly reported issues

[tdeekens]

This PR fixes the issue of nesting flags in the store state. In short it was a silly regression from the TypeScript migration. Sorry about that. I added more tests for it to not happen again.

I will follow up with subsequent PRs to adress other things too:

1. Properly test the store in integration (not using redux-mock-store)

You furthermore mentioned:

I found that memory-adapter has an action creator (/packages/react-redux/modules/ducks/flags/flags.ts@master#L38) that is unfortunately not exported by the module (/packages/react-redux/modules/index.ts@master) although docs say it does (/readme.md@master#L262)

There is some confusion here I think. An adapter can, but doesn't have to, expose an updateFlags method import { updateFlags } from '@flopflip/localstorage-adapter'. This method binds into the action creator of Redux when using @flopflip/react-redux. When using @flopflip/react-broadcast it just updates some component state. I would prefer not to actually export the redux action creator itself (from the ducks module). As it's an implementation detail of the package and then opens up two ways to update flags: through the adapater and the action creator. Adapters are actually the source of data and should allow editing it. In case of e.g. the @flopflip/lauchdarkly-adapter one would never update flags manually (but through their interface) hence the adapter does not expose an updateFlags. Updating them then in the store through an updateFlags action creator can lead to hard to debug side-effects when the change is overwritten again with another sync with the LaunchDarkly APIs. Hope this makes sense.

[montogeek]

@tdeekens Thank you so much for promptly fixing this bug, really appreciated it. About the updateFlags action creator, yes, it makes sense.

Would you recommend using

dispatch({
      type: UPDATE_FLAGS,
      payload: {
        flags: {
          socialLogin: {
            facebook: true,
            google: true
          }
        }
      }
    });

or an action creator to update flags when using memory-adapter?

Just trying to follow good practices.

Thanks again!

[tdeekens]

I'll release the fix soon and ping you here again.

To answer your question above. I would not use redux at all to upte flags. Neither an action creator (not exported) nor by dispatching an action myself. You can just

import { updateFlags } from '@flopflip/memory-adapter';

// some code passes
updateFlags({
  enableSocialLoginViaFacebook: true,
  enableSocialLoginViaGoogle: true,
})

the updateFlags (from the adapter) behind the scenes dispatches the action for you and updates the store state as needed. Furthermore, note that flags shoudl always be flattened. I never had the need to have a hierachy in flags.

[tdeekens]

Released @flopflip/react-redux@8.0.1. In a fork of your codesandbox here we can see that the state is now not nested anymore.

Thanks for reporting!

[montogeek]

Thanks to you!