feat[client-js]: Adds opt-in React Router loaders for @okta/okta-client-javascript - #321
feat[client-js]: Adds opt-in React Router loaders for @okta/okta-client-javascript#321BenjaminTruong-okta wants to merge 4 commits into
Conversation
…nt-javascript Adds a `@okta/okta-react/client-js` subpath exporting createFetchLoader, createTokenLoader, and createLoginCallbackLoader - React Router v6.4+ data-router loader factories that wrap @okta/auth-foundation, @okta/oauth2-flows, and @okta/spa-platform (0.6.0 beta) as an alternative to the okta-auth-js-based API. createLoginCallbackLoader resumes the auth code flow via AuthorizationCodeFlowOrchestrator.resumeFlow(), which exchanges the code and stores the resulting credential itself, then redirects to the original URI. This replaces the <LoginCallback /> component for apps using the new SDK's data router. The new peer SDKs are declared as optional peerDependencies so the default @okta/okta-react bundle has no dependency on them; a dedicated Rollup target and ESLint import-boundary rule keep the two bundles isolated from each other. Jest's jsdom environment doesn't implement a spec-compliant Fetch API, which the data router's loader/redirect handling depends on - added undici (plus its Node-native web API polyfills) via a new setupFiles entry so the loader tests exercise real Request/Response objects.
Expands the client-js section to contrast the authState/React-context model used elsewhere in this SDK with client-js's evaluate-at-point-of-use approach, including a before/after example and concrete differences around staleness, effect dependencies, bootstrapping, and subscriptions. Also fixes the SDK construction example: FetchClient takes a TokenOrchestrator as its first constructor argument, not a bare config object, so tokenOrchestrator must be constructed first.
5fdc0a7 to
0aca968
Compare
| export function createFetchLoader( | ||
| fetchClient: FetchClient, | ||
| getResource: GetResource, | ||
| init?: RequestInit, | ||
| ) { | ||
| return async (args: LoaderArgs): Promise<Response> => { | ||
| return fetchClient.fetch(getResource(args), init); | ||
| }; | ||
| } |
There was a problem hiding this comment.
| export function createFetchLoader( | |
| fetchClient: FetchClient, | |
| getResource: GetResource, | |
| init?: RequestInit, | |
| ) { | |
| return async (args: LoaderArgs): Promise<Response> => { | |
| return fetchClient.fetch(getResource(args), init); | |
| }; | |
| } | |
| export function createFetchLoader(fetchClient: FetchClient) { | |
| return async (url: string | URL | Request, init?: RequestInit): Promise<Response> => { | |
| return fetchClient.fetch(url, init); | |
| }; | |
| } |
| export function createTokenLoader( | ||
| orchestrator: AuthorizationCodeFlowOrchestrator, | ||
| params?: TokenOrchestrator.AuthorizeParams, | ||
| ) { | ||
| return async (): Promise<Token> => { | ||
| const token = await orchestrator.getToken(params); | ||
| if (!token) { | ||
| throw new Response('Unauthorized', { status: 401 }); | ||
| } | ||
| return token; | ||
| }; | ||
| } |
There was a problem hiding this comment.
I think the "create loader" should essentially just bind to an orchestrator instance. The params can be provided to the loader itself to override at the specific route
| export function createTokenLoader( | |
| orchestrator: AuthorizationCodeFlowOrchestrator, | |
| params?: TokenOrchestrator.AuthorizeParams, | |
| ) { | |
| return async (): Promise<Token> => { | |
| const token = await orchestrator.getToken(params); | |
| if (!token) { | |
| throw new Response('Unauthorized', { status: 401 }); | |
| } | |
| return token; | |
| }; | |
| } | |
| export function createTokenLoader( | |
| orchestrator: AuthorizationCodeFlowOrchestrator | |
| ) { | |
| return async (params?: TokenOrchestrator.AuthorizeParams): Promise<Token> => { | |
| const token = await orchestrator.getToken(params); | |
| if (!token) { | |
| throw new Response('Unauthorized', { status: 401 }); | |
| } | |
| return token; | |
| }; | |
| } |
There was a problem hiding this comment.
I made this change, but it requires the loader to be wrapped: (loader: () => fn(...)) instead of assigning them directly because they are no longer valid react router loaders on their own
| * | ||
| * See the License for the specific language governing permissions and limitations under the License. | ||
| */ | ||
|
|
There was a problem hiding this comment.
I wonder if it makes more sense to have a createLoadersFromOrchestrator method that returns both the tokenLoader and loginCallbackLoader since they both accept the same input (orchestrator) and they probably need to interact with the same orchestrator instance to work. Then recommend this pattern is used it instead
function createLoadersFromOrchestrator (orchestrator: AuthorizationCodeFlowOrchestrator) {
return {
tokenLoader: createTokenLoader(orchestrator),
loginCallbackLoader: createLoginCallbackLoader(orchestrator),
}
}
// usage
const { tokenLoader, loginCallbackLoader } = createLoadersFromOrchestrator(orchestrator)…all args createFetchLoader and createTokenLoader now take only the fetchClient/ orchestrator at creation time and return a function that takes the resource/params at call time, instead of baking those in upfront. The returned function no longer matches React Router's loader signature directly, so it must be called from within your own loader function. Adds createLoadersFromOrchestrator, which binds a single orchestrator instance to both createTokenLoader and createLoginCallbackLoader, since they need to share that instance to see each other's stored credential. Addresses review feedback on #321.
| path: '/', | ||
| element: <Page />, | ||
| loader: createFetchLoader(fetchClient as any, () => '/api/resource'), | ||
| loader: () => fetchResource('/api/resource'), |
There was a problem hiding this comment.
see here
|
This looks good! I like the direction it's heading I was poking around the
It seems like |
|
@jaredperreault-okta i think these loaders are fundamentally incompatible with declarative mode, where the |
|
|
||
| import Enzyme from 'enzyme'; | ||
| import Adapter from 'enzyme-adapter-react-16'; | ||
| import { fetch, Headers, Request, Response } from 'undici'; |
There was a problem hiding this comment.
You shouldn't need an external dependency for this. You can use the types available from node.
see https://github.com/okta/okta-client-javascript/tree/master/tooling/jest-helpers/browser
2 participants
Adds a
@okta/okta-react/client-jssubpath exporting createFetchLoader, createTokenLoader, and createLoginCallbackLoader - React Router v6.4+ data-router loader factories that wrap @okta/auth-foundation, @okta/oauth2-flows, and @okta/spa-platform (0.6.0 beta) as an alternative to the okta-auth-js-based API.createLoginCallbackLoader resumes the auth code flow via AuthorizationCodeFlowOrchestrator.resumeFlow(), which exchanges the code and stores the resulting credential itself, then redirects to the original URI. This replaces the component for apps using the new SDK's data router.
The new peer SDKs are declared as optional peerDependencies so the default @okta/okta-react bundle has no dependency on them; a dedicated Rollup target and ESLint import-boundary rule keep the two bundles isolated from each other.
Jest's jsdom environment doesn't implement a spec-compliant Fetch API, which the data router's loader/redirect handling depends on - added undici (plus its Node-native web API polyfills) via a new setupFiles entry so the loader tests exercise real Request/Response objects.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Reviewers