[SDK-1580] [SDK-1581] Add withAuth and withLoginRequired #7
Conversation
adamjmcgrath
added
the
medium
| @@ -28,7 +28,7 @@ Auth0 SDK for React Applications. | |||
| ## Installation | |||
|
|
|||
| ```bash | |||
| npm install react react-dom @auth0/auth0-spa-js auth0/auth0-react | |||
| npm install @auth0/auth0-spa-js auth0/auth0-react | |||
There was a problem hiding this comment.
I thought we didn't really need to tell users to install react
There was a problem hiding this comment.
Could removing this though lead to the assumption that we install react for them as a dependency? How would they know that without diving into package.json?
Perhaps we don't need to include it in the installation command but we could cover that with a quick sentence prior to this which says something to the effect of "This SDK should be installed into a React application". The clue is in the name. However, I think we should be clear.
| @@ -20,7 +20,6 @@ | |||
| "types": "dist/index.d.ts", | |||
| "module": "dist/auth0-react.esm.js", | |||
| "scripts": { | |||
| "prepare": "rollup -c --environment NODE_ENV:production", | |||
There was a problem hiding this comment.
This doesn't work with react because of facebook/react#14257
adamjmcgrath
changed the title
| @@ -28,7 +28,7 @@ Auth0 SDK for React Applications. | |||
| ## Installation | |||
|
|
|||
| ```bash | |||
| npm install react react-dom @auth0/auth0-spa-js auth0/auth0-react | |||
| npm install @auth0/auth0-spa-js auth0/auth0-react | |||
There was a problem hiding this comment.
Could removing this though lead to the assumption that we install react for them as a dependency? How would they know that without diving into package.json?
Perhaps we don't need to include it in the installation command but we could cover that with a quick sentence prior to this which says something to the effect of "This SDK should be installed into a React application". The clue is in the name. However, I think we should be clear.
| * Get an access token. | ||
| */ | ||
| getToken: ( | ||
| options?: GetTokenSilentlyOptions |
There was a problem hiding this comment.
If we're surfacing options types from SPA JS here are we still able to generate documentation as if they were part of this library?
Surfacing these options couples us tighter to the underlying SDK, which should be somewhat transparent.
There was a problem hiding this comment.
True, but I think we'd need to bundle a version of spa js with the library if we don't do this (which we could do?)
There was a problem hiding this comment.
It's worth a further discussion but all I'm talking about really is wrapping those SDK types with types native to the React library rather than exposing GetTokenSilentlyOptions.
We would just have GetTokenOptions for example, which internally maps to GetTokenSilentlyOptions. Means the SDK isn't as exposed but also we can generate the right doctypes and not force the user to go elsewhere to figure out what properties they can specify and what their docs are.
adamjmcgrath
changed the title
Description
Adds higher order components:
withAuthfor wrapping class componentswithLoginRequiredfor protecting components (usually routes)Testing
Checklist
master