WIP - Add react package

[AlfredoGJ]

What does this PR do?

Address #28

Provides an access-decision-manager binding for react with the next modules

<AccessDecisionManagerProvider user={} voters={}> component - Provides a component that binds the accessDecisionManager functioality with their respective arguments {user, voters, context?

useIsGranted() - hook- This hooks must be used in components that are under the AccessDecisionManagerProvider component to be able to use the voters and user

Why is this important?

Where should the reviewer start?

How should this be manually tested?

1. Clone the repo
2. Go to /packages/access-decision-manager-react/
3. run npm install
4. run npx jest --verbose all test should pass

Pending work

• Create readme
• Enable strategies
• Verify usage of context
• Write examples with tests

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have added necessary documentation (if appropriate)
• I did review existing Pull Requests before submitting mine
• I have added tests that prove my fix is effective or that my feature works

[m14t]

Note: I started merging some of the NPM package updates and now the package-lock.json file needs to be updated. I think it should be relatively easy to rebase this branch accepting their changes, and then running npm update to fix.

[m14t]

Let's make sure to not import things we don't use.

Ideally we have an ESLint rule to check for this.

[m14t]

Let's not prefix our Interfaces with I. See typescript-eslint/typescript-eslint#374 (comment) for an explaination.

Ideally we should have that ESLint rule enabled to check this for us.

[m14t]

Discussion: I could be wrong, but I think we might be better off using useRef here instead of useState. useState will always cause a rerender, where instead I think we want to only rerender the consumers of our context.

[AlfredoGJ]

It makes me think a bit... Maybe accessDecisionManager could be just a var since the AccessDecisionManagerProvider component does not need to preserve its value across renders (that is done by AccessDecisionManagerContext.Provider component already) and we only need to change it when user, voters or createContext are updated

[AlfredoGJ]

Update: yeah it needs to be a ref, since a var will be re-created every time the component renders even if the user voters or createContext props keep the same

[AlfredoGJ]

I left this as a useState cause with useRef creating a new accessDecisionManager on user voters or createContext properties update had no effect on AccessDecisionManagerContext.Provider's value

[m14t]

I like this approach. It reminds me of https://kentcdodds.com/blog/stop-using-isloading-booleans

[m14t]

Maybe we can improve debugging if we also tell the action type:

      throw new Error(`Unhandled action type: ${action.type}`)

[AlfredoGJ]

Nice catch, will improve it

[m14t]

Let's improve the types here to not use any.

[m14t]

This is a preference and we should have an ESLint rule for it, but lets always use curly braces with our if statements:

https://eslint.org/docs/rules/curly

[m14t]

We can discuss this more later, but having multiple "empty values" (undefined, and null) can be confusing and so far I think we've been consistent on using undefined.

If possible, let's try to stick to only using undefined when a value is not set.

[AlfredoGJ]

Personally, i think in some cases null is a more descriptive value than undefined but im ok with keep using undefined for the sake of consistency