feat!: Add SecureOutlet, split SecureRoute/SecureOutlet into router-specific sub-exports - #326
feat!: Add SecureOutlet, split SecureRoute/SecureOutlet into router-specific sub-exports#326BenjaminTruong-okta wants to merge 4 commits into
Conversation
SecureRoute and SecureOutlet are v5- and v6-shaped code respectively, but both previously lived in the same top-level @okta/okta-react bundle/module graph. That meant react-router-dom symbols referenced by either component were a latent bundler/type risk for every consumer, regardless of which router version (or neither) they used - this already caused a real production build break (#178/#187, patched in #210/#213). Move SecureRoute to @okta/okta-react/react-router-5 and SecureOutlet to @okta/okta-react/react-router-6, each built as its own bundle, so unused router-version code never enters a consumer's build graph. Ports the design from the unmerged #282 (dev7) onto current master's tooling. BREAKING CHANGE: SecureRoute and SecureOutlet are no longer exported from @okta/okta-react. Import SecureRoute from @okta/okta-react/react-router-5 and SecureOutlet from @okta/okta-react/react-router-6 instead. Minimum supported Node version is now 12.17.0.
| const OktaContext = React.createContext<IOktaContext | null>(null); | ||
|
|
||
| export const useOktaAuth = (): IOktaContext => React.useContext(OktaContext) as IOktaContext; | ||
| export const useOktaAuth = (context?: typeof OktaContext): IOktaContext => React.useContext(context ?? OktaContext) as IOktaContext; |
There was a problem hiding this comment.
Do you know why this is required? I get why you're doing this, just seems a bit awkward
There was a problem hiding this comment.
so each bundle can supply the singleton instance to useOktaAuth instead of useOktaAuth defaulting to a local OktaContext
| import { useOktaAuth, OnAuthRequiredFunction } from './OktaContext'; | ||
| import * as ReactRouterDom from 'react-router-dom'; | ||
| import { toRelativeUrl, AuthSdkError } from '@okta/okta-auth-js'; | ||
| // Important! Don't import OktaContext from './OktaContext' |
There was a problem hiding this comment.
You should be able to add an eslint rule (like this) to prevent this
| import { useOktaAuth, OnAuthRequiredFunction } from './OktaContext'; | ||
| import * as ReactRouterDom from 'react-router-dom'; | ||
| import { toRelativeUrl, AuthSdkError } from '@okta/okta-auth-js'; | ||
| // Important! Don't import OktaContext from './OktaContext' |
There was a problem hiding this comment.
Same as above, eslint rule
| const external = makeExternalPredicate(); | ||
| const commonPlugins = [ | ||
|
|
||
| // Each build below needs its own `typescript()` plugin instance (with its own cacheRoot). |
There was a problem hiding this comment.
I migrated away from rollup-plugin-typescript2 in okta-client-javascript. May be worth investigating (example: https://github.com/okta/okta-client-javascript/blob/master/tooling/rollup-config/sdk.mjs#L4). This explanation seems very nuanced and indicates this may be potentially fragile
rollup-plugin-typescript2 emitting .d.ts files from multiple entry-point configs (one per sub-export) fought over writing to the same declarationDir, requiring per-config cache roots and scratch-path redirects to avoid TS5055. Decouple declaration emission from bundling entirely: rollup's typescript() plugin now only transpiles, and a single `yarn types` (tsc --emitDeclarationOnly) pass emits every .d.ts file in one whole-program compile, matching the pattern used in okta-client-javascript's rollup config. skipLibCheck is added to the root tsconfig since running tsc directly now surfaces a pre-existing duplicate @types/react conflict from workspace hoisting that rollup-plugin-typescript2 wasn't checking.
Add a no-restricted-imports override blocking the default OktaContext export from './OktaContext' in these two files, so the "import from @okta/okta-react instead" requirement is enforced by lint rather than relying on a comment being followed.
SecureRoute only worked with react-router-dom v5 APIs, forcing v6+ users to hand-roll their own guard component (see issues #267, #300). This PR adds
SecureOutlet, which mirrorsSecureRoute's auth-gating behavior usingOutlet, and then splits both components into router-version-specific sub-exports so unused router code never enters a consumer's bundle.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?
SecureRouteonly works withreact-router-domv5 (Route-based) APIs. v6+ apps, which useOutlet-based nested routing, have no built-in guard component and must hand-roll one (#267, #300). Additionally,SecureRouteliving in the top-level@okta/okta-reactbundle meansreact-router-domsymbols it references are a latent bundler/type risk for every consumer, regardless of whetherSecureRouteis used — this already caused a real production build break (#178/#187, patched reactively in #210/#213).Issue Number: #267, #300
What is the new behavior?
SecureOutlet, a v6+ equivalent ofSecureRoutebuilt onOutlet, with the same auth-gating behavior.@okta/okta-reactexport into dedicated sub-exports:SecureRoute→@okta/okta-react/react-router-5,SecureOutlet→@okta/okta-react/react-router-6. Each is built as its own bundle viapackage.jsonexports, so router-version-specific code is only pulled into a consumer's build when they actually import it — closing the whole class of bug behind Support react-router v6 #178/'useRouteMatch' is not exported from 'react-router-dom' #187 rather than patching it symbol-by-symbol.okta/okta-react#282(dev7) draft onto current tooling.react-router-dombecomes an optional peer dependency.Does this PR introduce a breaking change?
SecureRouteandSecureOutletare no longer exported from@okta/okta-react. Consumers must update imports:Minimum supported Node version is now
12.17.0(required for package self-referencing viaexports). Major version bump:6.12.0→7.0.0.Other information
See
CHANGELOG.mdand the updatedREADME.md"Upgrading to7.x" note for full migration details.Reviewers