Skip to content

Commit

Permalink
fix: Prevents useEffect double render in React18 StrictMode (#266)
Browse files Browse the repository at this point in the history
OKTA-635977 Prevents useEffect double render in React18 StrictMode
  • Loading branch information
trevoring-okta authored Sep 5, 2023
1 parent 9fd00ae commit 335defc
Show file tree
Hide file tree
Showing 21 changed files with 2,320 additions and 1,449 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 6.8.0

### Bug Fixes

-[#266](https://github.com/okta/okta-react/pull/266) Fixes useEffect double render in React18 StrictMode

# 6.7.0

-[#248](https://github.com/okta/okta-react/pull/248) Adds support for `@okta/okta-auth-js` `7.x`
Expand Down
18 changes: 10 additions & 8 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ module.exports = {
AUTH_JS: { minSupportedVersion: '5.3.1' },
PACKAGE_NAME: 'okta-react-test',
PACKAGE_VERSION: '3.14.15',
SKIP_VERSION_CHECK: '1',
'ts-jest': {
diagnostics: {
warnOnly: true
},
tsconfig: '<rootDir>/test/jest/tsconfig.json'
}
SKIP_VERSION_CHECK: '1'
},
reporters: [
'default',
Expand All @@ -35,5 +29,13 @@ module.exports = {
'./test/jest/setup.ts'
],
testEnvironment: 'jsdom',
transform: { '^.+\\.tsx?$': 'ts-jest' },
transform: {
'^.+\\.tsx?$': ['ts-jest', {
diagnostics: {
warnOnly: true
},
tsconfig: '<rootDir>/test/jest/tsconfig.json',
isolatedModules: true
}]
},
};
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"@testing-library/react": "^12.1.3",
"@types/enzyme": "^3.10.8",
"@types/enzyme-adapter-react-16": "^1.0.6",
"@types/jest": "^27.4.1",
"@types/jest": "^29.5.4",
"@types/react": "^16.9.0",
"@types/react-dom": "^16.9.0",
"@types/react-router-dom": "^5.1.6",
Expand All @@ -91,7 +91,8 @@
"globby": "^11.0.1",
"jasmine-core": "~2.6.2",
"jasmine-spec-reporter": "~4.1.0",
"jest": "^27.5.1",
"jest": "^29.6.4",
"jest-environment-jsdom": "^29.6.4",
"jest-junit": "^13.0.0",
"polished": "^1.7.0",
"prop-types": "^15.5.10",
Expand All @@ -104,7 +105,7 @@
"rollup-plugin-terser": "^7.0.2",
"rollup-plugin-typescript2": "^0.29.0",
"shelljs": "^0.8.5",
"ts-jest": "^27.1.4",
"ts-jest": "^29.1.1",
"typescript": "^4.0.5"
},
"jest-junit": {
Expand Down
2 changes: 1 addition & 1 deletion scripts/lint.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash -xe
#!/bin/bash -x

source ${OKTA_HOME}/${REPO}/scripts/setup.sh

Expand Down
2 changes: 1 addition & 1 deletion scripts/publish.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash -xe
#!/bin/bash -x

source ${OKTA_HOME}/${REPO}/scripts/setup.sh

Expand Down
2 changes: 1 addition & 1 deletion scripts/setup.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash -xe
#!/bin/bash -x

DIR=$(cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd)
source $DIR/utils/local.sh
Expand Down
42 changes: 0 additions & 42 deletions scripts/update-se-drivers.js

This file was deleted.

13 changes: 10 additions & 3 deletions src/LoginCallback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ interface LoginCallbackProps {
loadingElement?: React.ReactElement;
}

let handledRedirect = false;

const LoginCallback: React.FC<LoginCallbackProps> = ({ errorComponent, loadingElement = null, onAuthResume }) => {
const { oktaAuth, authState } = useOktaAuth();
const [callbackError, setCallbackError] = React.useState(null);
Expand All @@ -33,9 +35,14 @@ const LoginCallback: React.FC<LoginCallbackProps> = ({ errorComponent, loadingEl
onAuthResume();
return;
}
oktaAuth.handleLoginRedirect().catch(e => {
setCallbackError(e);
});
// OKTA-635977: Prevents multiple calls of handleLoginRedirect() in React18 StrictMode
// Top-level variable solution follows: https://react.dev/learn/you-might-not-need-an-effect#initializing-the-application
if (!handledRedirect) {
oktaAuth.handleLoginRedirect().catch(e => {
setCallbackError(e);
})
handledRedirect = true;
}
}, [oktaAuth]);

const authError = authState?.error;
Expand Down
2 changes: 2 additions & 0 deletions test/apps/test-harness-app/.env.development
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Forces Vite to run a development build: https://vitejs.dev/guide/env-and-mode.html#modes
NODE_ENV=development
12 changes: 7 additions & 5 deletions test/apps/test-harness-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,23 @@
"version": "0.0.0",
"scripts": {
"prestart": "vite build",
"start": "vite preview --port 8080",
"start": "vite preview --port 8080",
"start:dev": "vite build --mode development && vite preview --port 8080",
"dev": "vite",
"build": "tsc && vite build",
"preview": "vite preview"
},
"dependencies": {
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-router-dom": "5.2.0",
"@okta/okta-auth-js": "*",
"@okta/okta-react": "*"
},
"devDependencies": {
"@types/react": "^17.0.33",
"@types/react-dom": "^17.0.10",
"@types/react": "^18.2.15",
"@types/react-dom": "^18.2.7",
"@types/react-router-dom": "^5.3.3",
"@vitejs/plugin-react": "^1.3.2",
"typescript": "^4.5.4",
"vite": "^2.8.0"
Expand Down
8 changes: 8 additions & 0 deletions test/apps/test-harness-app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ const App: React.FC<{
{...props}
onAuthResume={ onAuthResume }
loadingElement={ <p id='login-callback-loading'>Loading...</p> }
errorComponent={(props: any) => {
const { error } = props;
return (
<p id='login-callback-error'>
{error?.name}:{error?.message}
</p>
);
}}
/>
} />
<Route path='/' component={Home} />
Expand Down
7 changes: 4 additions & 3 deletions test/apps/test-harness-app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { createRoot } from 'react-dom/client';
import { OktaAuth } from '@okta/okta-auth-js';
import './index.css';
import App from './App';
Expand All @@ -35,9 +35,10 @@ const oktaAuth = new OktaAuth({
pkce
});

ReactDOM.render(
const container = document.getElementById('root')

createRoot(container!).render(
<Router>
<App oktaAuth={oktaAuth} customLogin={customLogin} baseUrl={baseUrl} />
</Router>
, document.getElementById('root')
);
2 changes: 2 additions & 0 deletions test/apps/v6-app/.env.development
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Forces Vite to run a development build: https://vitejs.dev/guide/env-and-mode.html#modes
NODE_ENV=development
10 changes: 6 additions & 4 deletions test/apps/v6-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@
"scripts": {
"prestart": "vite build",
"start": "vite preview --port 8080",
"start:dev": "vite build --mode development && vite preview --port 8080",
"dev": "vite",
"build": "tsc && vite build",
"preview": "vite preview"
},
"dependencies": {
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-router-dom": "6.3.0",
"@okta/okta-auth-js": "*",
"@okta/okta-react": "*"
},
"devDependencies": {
"@types/react": "^17.0.33",
"@types/react-dom": "^17.0.10",
"@types/react": "^18.2.15",
"@types/react-dom": "^18.2.7",
"@types/react-router-dom": "^5.3.3",
"@vitejs/plugin-react": "^1.3.2",
"typescript": "^4.5.4",
"vite": "^2.8.0"
Expand Down
21 changes: 17 additions & 4 deletions test/apps/v6-app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,23 @@ const App: React.FC<{
<Route path='' element={<Protected />} />
</Route>
<Route path='/implicit/callback' element={<LoginCallback />} />
<Route path='/pkce/callback' element={<LoginCallback
onAuthResume={ onAuthResume }
loadingElement={ <p id='login-callback-loading'>Loading...</p> }
/>}/>
<Route
path='/pkce/callback'
element={
<LoginCallback
onAuthResume={ onAuthResume }
loadingElement={ <p id='login-callback-loading'>Loading...</p> }
errorComponent={(props: any) => {
const { error } = props;
return (
<p id='login-callback-error'>
{error?.name}:{error?.message}
</p>
);
}}
/>
}
/>
<Route path='/' element={<Home />} />
</Routes>
</Security>
Expand Down
7 changes: 4 additions & 3 deletions test/apps/v6-app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { createRoot } from 'react-dom/client';
import { OktaAuth } from '@okta/okta-auth-js';
import './index.css';
import App from './App';
Expand All @@ -35,9 +35,10 @@ const oktaAuth = new OktaAuth({
pkce
});

ReactDOM.render(
const container = document.getElementById('root')

createRoot(container!).render(
<Router>
<App oktaAuth={oktaAuth} customLogin={customLogin} baseUrl={baseUrl} />
</Router>
, document.getElementById('root')
);
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
class LoginCallbackPage {

get loadingElement () { return $('#login-callback-loading'); }
get errorElement () { return $('#login-callback-error'); }

waitForPageLoad() {
return this.loadingElement.waitForDisplayed();
Expand Down
9 changes: 6 additions & 3 deletions test/e2e/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ function runNextTask(tasks) {

function runWithConfig(sampleConfig) {
return new Promise((resolve) => {
const { name } = sampleConfig;
const { name, mode } = sampleConfig;
const port = sampleConfig.port || 8080;
// OKTA-635977: Adds a mode flag to configs to determine whether to build in prod or dev, can be extended from a ternary
// if testing against other build modes becomes necessary. Defaults to building in prod via 'yarn start'
const command = mode ? `start:${mode}` : 'start'

// 1. start the sample's web server
const server = spawn('yarn', [
'workspace',
name,
'start'
command
], { stdio: 'inherit' });

waitOn({
Expand Down Expand Up @@ -79,7 +82,7 @@ function runHarnessTests () {
const config = {
name: '@okta/test.app.test-harness-app',
specs: ['test-harness-app'],
port: 8080
mode: 'dev'
}

return runWithConfig(config);
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/specs/test-harness-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,29 @@ describe('React + Okta App', () => {
});
});

describe('React18 StrictMode double render', () => {
it('should only call handleLoginRedirect() once on render', async () => {
await AppPage.open('/?pkce=1');

await AppPage.waitForPageLoad();
expect(await AppPage.loginFlow.getText()).toBe('PKCE');
await AppPage.loginButton.click();

await OktaSignInPage.waitForPageLoad();
await OktaSignInPage.login(USERNAME, PASSWORD);

// OKTA-635977: Expect that the loading element in LoginCallback gets rendered to the page instead of the ErrorComponent
await LoginCallbackPage.waitForPageLoad();
expect (await LoginCallbackPage.errorElement.waitForExist({ reverse: true })).toBeTruthy();

await AppPage.waitForPageLoad();
expect(await AppPage.logoutButton.isExisting()).toBeTruthy();

await AppPage.logoutButton.click();
await AppPage.waitForLogout();
})
})

describe('Okta session token flow', () => {

it('should allow passing sessionToken to skip Okta login', async () => {
Expand Down
Loading

0 comments on commit 335defc

Please sign in to comment.