search

syncweek-react-aad / react-aad

A React wrapper for Azure AD using the Microsoft Authentication Library (MSAL). The easiest way to integrate AzureAD with your React for authentication.
MIT License
345 stars 94 forks source link

Decouple the Redux logic into a new project #155

Open AndrewCraswell opened 4 years ago

AndrewCraswell commented 4 years ago

Is your feature request related to a problem? Please describe. The logic for tracking authentication progress, success, and failure and dispatching Redux events to store the info into a Redux store has become very tightly coupled to the authentication logic itself. This is bad for the general complexity of the library and means there's lots of extra code that is not relevant to users who aren't utilizing Redux at all.

It's also true that you don't need the AzureAD component to track these state changes. All the necessary callbacks are already exposed on the MSAL UserAgentApplication class, so state management should be tied directly to the MSAL library.

Describe the solution you'd like All Redux state management logic should be removed and placed into a separate library whose sole purpose is to track MSAL changes against a Redux store. The adapter plugin could automatically hook into the MSAL event callbacks and push relevant data into the Redux store.

This would allow the react-aad-msal library to be much smaller and less complex and not rely on Redux as a dependency. It would also provide some more usefulness to anyone using MSAL with other wrapper libraries or even vanilla JS apps who also want to utilize Redux to track state.

Additional context Ultimately, anyone wanting to use a third party state management system should create a separate adapter plugin that could closely match the Redux plugin described above. It could also be possible to develop a shared interface for building state management adapters against MSAL so that other state libraries (RxJs, MobX, etc) could easily be integrated.

NathanNZ commented 4 years ago

It seems the Redux Store is only used for dispatching actions, and I don't see any benefit with storing internal state within the store in future. Do you think it would be an acceptable suggestion to replace the store prop on the AzureAD component with a generic dispatch method?

This could allow people who are using Redux or other patterns that use actions or event based logic flows to pass in their dispatch function directly. I believe this would remove the dependency on the Redux library and resolve the issues mentioned without changing the status quo up too much for those who already have hooked this up to their state management system. There is also the minor benefit of reducing the amount of packages that those who use state libraries will need to install and audit (no need for a react-aad-redux library etc).

AndrewCraswell commented 4 years ago

@NathanNZ I really like this idea. Should be relatively easy to implement too.

pvaillant commented 3 years ago

Has there been any progress in removing the Redux dependency? I have a project that I would like to use this library in but that isn't using Redux and where it's going to be hard to justify bringing Redux in just for this.

If this isn't started yet, or if I can help, I'd be happy to put some effort towards this.