feat: skip unnecessary updates in SessionAuth (#831)

* feat: skip unnecessary updates in SessionAuth * chore: update version * chore: update size limit
supertokens · Jun 25, 2024 · 9ed9ce8 · 9ed9ce8
1 parent f43dd39
commit 9ed9ce8
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,11 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [unreleased]
 
-## [0.42.3] - 2024-06-20
+## [0.43.0] - 2024-06-25
 
 ### Changes
 
--   Update SAML third-party provider to use logo based on the provider name
+-   Updated the SAML third-party provider to use logo based on the provider name
+
+## [0.42.3] - 2024-06-24
+
+### Changes
+
+-   Now we only update the session context if the object changes by value. This optimization should help reduce unnecessary re-renders.
 
 ## [0.42.2] - 2024-05-29
 

diff --git a/examples/for-tests-react-16/src/App.js b/examples/for-tests-react-16/src/App.js
@@ -418,6 +418,14 @@ export const DashboardNoAuthRequired = doNotUseReactRouterDom
 export function DashboardNoAuthRequiredHelper(props) {
     let sessionContext = useSessionContext();
 
+    useEffect(() => {
+        if (testContext.signoutOnSessionNotExists) {
+            if (!sessionContext.loading && !sessionContext.doesSessionExist) {
+                Session.signOut();
+            }
+        }
+    }, [sessionContext]);
+
     if (sessionContext.loading) {
         return null;
     }

diff --git a/examples/for-tests-react-16/src/testContext.js b/examples/for-tests-react-16/src/testContext.js
@@ -21,6 +21,7 @@ export function getTestContext() {
                 : undefined,
         enableMFA: localStorage.getItem("enableMFA") === "true",
         defaultToEmail: localStorage.getItem("defaultToEmail") !== "false",
+        signoutOnSessionNotExists: localStorage.getItem("signoutOnSessionNotExists") === "true",
         disableRedirectionAfterSuccessfulSignInUp:
             localStorage.getItem("disableRedirectionAfterSuccessfulSignInUp") === "true",
     };

diff --git a/examples/for-tests/src/App.js b/examples/for-tests/src/App.js
@@ -590,6 +590,14 @@ export const DashboardNoAuthRequired = doNotUseReactRouterDom
 export function DashboardNoAuthRequiredHelper(props) {
     let sessionContext = useSessionContext();
 
+    useEffect(() => {
+        if (testContext.signoutOnSessionNotExists) {
+            if (!sessionContext.loading && !sessionContext.doesSessionExist) {
+                Session.signOut();
+            }
+        }
+    }, [sessionContext]);
+
     if (sessionContext.loading) {
         return null;
     }

diff --git a/examples/for-tests/src/testContext.js b/examples/for-tests/src/testContext.js
@@ -22,6 +22,7 @@ export function getTestContext() {
         },
         enableMFA: localStorage.getItem("enableMFA") === "true",
         defaultToEmail: localStorage.getItem("defaultToEmail") !== "false",
+        signoutOnSessionNotExists: localStorage.getItem("signoutOnSessionNotExists") === "true",
         disableRedirectionAfterSuccessfulSignInUp:
             localStorage.getItem("disableRedirectionAfterSuccessfulSignInUp") === "true",
     };

diff --git a/lib/build/index2.js b/lib/build/index2.js
diff --git a/lib/ts/recipe/session/sessionAuth.tsx b/lib/ts/recipe/session/sessionAuth.tsx
@@ -68,6 +68,22 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
     // Reusing the parent context was removed because it caused a redirect loop in an edge case
     // because it'd also reuse the invalid claims part until it loaded.
     const [context, setContext] = useState<SessionContextType>({ loading: true });
+    const setContextIfChanged = useCallback(
+        (newValue: SessionContextType) => {
+            setContext((oldValue) => {
+                // We can't do this check before re-validation because there are be validators that depend on the current time
+                // Since the context is constructed by the same functions the property order should be stable, meaning that
+                // a simple JSON string check should be sufficient.
+                // Plus since this is just an optimization it is fine to have false positives,
+                // and this method won't have false negatives (where we'd miss an update).
+                if (JSON.stringify(oldValue) !== JSON.stringify(newValue)) {
+                    return newValue;
+                }
+                return oldValue;
+            });
+        },
+        [setContext]
+    );
 
     const session = useRef<Session>();
 
@@ -191,7 +207,7 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
 
                         if (failureRedirectInfo.redirectPath !== undefined) {
                             if (validateAndCompareOnFailureRedirectionURLToCurrent(failureRedirectInfo.redirectPath)) {
-                                setContext(toSetContext);
+                                setContextIfChanged(toSetContext);
                                 return;
                             } else {
                                 return await SuperTokens.getInstanceOrThrow().redirectToUrl(
@@ -209,15 +225,15 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
                             message: "Showing access denied screen because a claim validator failed",
                             claimValidationError: failureRedirectInfo.failedClaim,
                         });
-                        return setContext({
+                        return setContextIfChanged({
                             ...toSetContext,
                             accessDeniedValidatorError: failureRedirectInfo.failedClaim,
                         });
                     }
                 }
             }
 
-            setContext(toSetContext);
+            setContextIfChanged(toSetContext);
         },
         [
             context.loading,
@@ -262,7 +278,7 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
                                 if (
                                     validateAndCompareOnFailureRedirectionURLToCurrent(failureRedirectInfo.redirectPath)
                                 ) {
-                                    setContext({ ...event.sessionContext, loading: false, invalidClaims });
+                                    setContextIfChanged({ ...event.sessionContext, loading: false, invalidClaims });
                                 } else {
                                     return await SuperTokens.getInstanceOrThrow().redirectToUrl(
                                         failureRedirectInfo.redirectPath,
@@ -279,23 +295,23 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
                                 message: "Showing access denied screen because a claim validator failed",
                                 claimValidationError: failureRedirectInfo.failedClaim,
                             });
-                            return setContext({
+                            return setContextIfChanged({
                                 ...event.sessionContext,
                                 loading: false,
                                 invalidClaims,
                                 accessDeniedValidatorError: failureRedirectInfo.failedClaim,
                             });
                         }
                     }
-                    setContext({ ...event.sessionContext, loading: false, invalidClaims });
+                    setContextIfChanged({ ...event.sessionContext, loading: false, invalidClaims });
 
                     return;
                 }
                 case "SIGN_OUT":
-                    setContext({ ...event.sessionContext, loading: false, invalidClaims: [] });
+                    setContextIfChanged({ ...event.sessionContext, loading: false, invalidClaims: [] });
                     return;
                 case "UNAUTHORISED":
-                    setContext({ ...event.sessionContext, loading: false, invalidClaims: [] });
+                    setContextIfChanged({ ...event.sessionContext, loading: false, invalidClaims: [] });
                     if (props.onSessionExpired !== undefined) {
                         props.onSessionExpired();
                     } else if (props.requireAuth !== false && props.doRedirection !== false) {
@@ -316,7 +332,7 @@ const SessionAuth: React.FC<PropsWithChildren<SessionAuthProps>> = ({ children,
             return session.current.addEventListener(onHandleEvent);
         }
         return undefined;
-    }, [props, setContext, context.loading, userContext, navigate, redirectToLogin]);
+    }, [props, setContextIfChanged, context.loading, userContext, navigate, redirectToLogin]);
 
     if (props.requireAuth !== false && (context.loading || !context.doesSessionExist)) {
         return null;

diff --git a/package.json b/package.json
@@ -137,7 +137,7 @@
     "size-limit": [
         {
             "path": "lib/build/index.js",
-            "limit": "23kb"
+            "limit": "23.5kb"
         },
         {
             "path": "recipe/session/index.js",

diff --git a/test/end-to-end/signin.test.js b/test/end-to-end/signin.test.js
@@ -213,6 +213,32 @@ describe("SuperTokens SignIn", function () {
             await waitForSTElement(page, "[data-supertokens~=generalError]", true);
         });
 
+        it("should work visiting no required session page", async function () {
+            consoleLogs = await clearBrowserCookiesWithoutAffectingConsole(page, consoleLogs);
+            let cookies = await page.cookies();
+            assert.deepStrictEqual(cookies.length, 1);
+            assert.deepStrictEqual(cookies[0].name, "st-last-access-token-update");
+            await page.evaluate(() => window.localStorage.setItem("signoutOnSessionNotExists", "true"));
+            await Promise.all([
+                page.goto(`${TEST_CLIENT_BASE_URL}/dashboard-no-auth`),
+                page.waitForNavigation({ waitUntil: "networkidle0" }),
+            ]);
+
+            let text = await getTextInDashboardNoAuth(page);
+
+            assert.strictEqual(text, "Not logged in");
+            // We wait for some time to ensure we didn't get into a sign-out loop
+            await new Promise((res) => setTimeout(res, 1000));
+            assert.deepStrictEqual(consoleLogs, [
+                "ST_LOGS SESSION OVERRIDE ADD_FETCH_INTERCEPTORS_AND_RETURN_MODIFIED_FETCH",
+                "ST_LOGS SESSION OVERRIDE ADD_AXIOS_INTERCEPTORS",
+                "ST_LOGS SESSION OVERRIDE SIGN_OUT",
+                "ST_LOGS SESSION ON_HANDLE_EVENT SIGN_OUT",
+                "ST_LOGS SESSION OVERRIDE SIGN_OUT",
+                "ST_LOGS SESSION ON_HANDLE_EVENT SIGN_OUT",
+            ]);
+        });
+
         it("Successful Sign In with no required session page", async function () {
             await toggleSignInSignUp(page);
             await defaultSignUp(page);
@@ -492,8 +518,7 @@ describe("SuperTokens SignIn", function () {
                 ),
                 page.waitForNavigation({ waitUntil: "networkidle0" }),
             ]);
-            const { pathname, search } = await page.evaluate(() => window.location);
-            assert.deepStrictEqual(pathname + search, "/redirect-heree?foo=bar");
+            await waitForUrl(page, "/redirect-heree?foo=bar", false);
         });
 
         it("Successful Sign In with redirect to, redirectToPath directly without trailing slash", async function () {