From 1c94aff5a01ed51d93be0819a829231618e77109 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Tue, 9 Jan 2024 04:29:16 +0200 Subject: [PATCH 1/3] feat(clerk-react,clerk-js,shared): Dynamically update props based on deep equality This change was made in an effort to support dynamic values for appearance, localization as modifying this values usually happens during dev where a fast HMR iteration cycle is required. However, we'd also like to support dynamic values for props like redirectUrl, where the value can be calculated based on data available during runtime, even after the Clerk components mount (eg, based on the results of a fetch request). In order to avoid extra renders, we use the pre-existing dequal utility to make sure that expensive calculations are avoided. --- .../clerk-js/src/ui/customizables/AppearanceContext.tsx | 3 +-- packages/clerk-js/src/ui/hooks/index.ts | 1 - packages/react/src/components/uiComponents.tsx | 6 ++---- packages/shared/src/react/hooks/index.ts | 1 + .../src/ui => shared/src/react}/hooks/useDeepEqualMemo.ts | 2 ++ 5 files changed, 6 insertions(+), 7 deletions(-) rename packages/{clerk-js/src/ui => shared/src/react}/hooks/useDeepEqualMemo.ts (94%) diff --git a/packages/clerk-js/src/ui/customizables/AppearanceContext.tsx b/packages/clerk-js/src/ui/customizables/AppearanceContext.tsx index 201edc9cab8..dfedab25078 100644 --- a/packages/clerk-js/src/ui/customizables/AppearanceContext.tsx +++ b/packages/clerk-js/src/ui/customizables/AppearanceContext.tsx @@ -1,7 +1,6 @@ -import { createContextAndHook } from '@clerk/shared/react'; +import { createContextAndHook, useDeepEqualMemo } from '@clerk/shared/react'; import React from 'react'; -import { useDeepEqualMemo } from '../hooks'; import type { AppearanceCascade, ParsedAppearance } from './parseAppearance'; import { parseAppearance } from './parseAppearance'; diff --git a/packages/clerk-js/src/ui/hooks/index.ts b/packages/clerk-js/src/ui/hooks/index.ts index 5679be23e38..06141362e1d 100644 --- a/packages/clerk-js/src/ui/hooks/index.ts +++ b/packages/clerk-js/src/ui/hooks/index.ts @@ -17,6 +17,5 @@ export * from './useSafeState'; export * from './useSearchInput'; export * from './useDebounce'; export * from './useScrollLock'; -export * from './useDeepEqualMemo'; export * from './useClerkModalStateParams'; export * from './useNavigateToFlowStart'; diff --git a/packages/react/src/components/uiComponents.tsx b/packages/react/src/components/uiComponents.tsx index ce9033f9014..67cc6b5f6b0 100644 --- a/packages/react/src/components/uiComponents.tsx +++ b/packages/react/src/components/uiComponents.tsx @@ -1,4 +1,5 @@ import { logErrorInDevMode } from '@clerk/shared'; +import { isDeeplyEqual } from '@clerk/shared/react'; import type { CreateOrganizationProps, OrganizationListProps, @@ -90,10 +91,7 @@ class Portal extends React.PureComponent { private portalRef = React.createRef(); componentDidUpdate(prevProps: Readonly) { - if ( - prevProps.props.appearance !== this.props.props.appearance || - prevProps.props?.customPages?.length !== this.props.props?.customPages?.length - ) { + if (!isDeeplyEqual(prevProps.props, this.props.props)) { this.props.updateProps({ node: this.portalRef.current, props: this.props.props }); } } diff --git a/packages/shared/src/react/hooks/index.ts b/packages/shared/src/react/hooks/index.ts index b0d9779bc8b..b55def59478 100644 --- a/packages/shared/src/react/hooks/index.ts +++ b/packages/shared/src/react/hooks/index.ts @@ -6,3 +6,4 @@ export { useSession } from './useSession'; export { useSessionList } from './useSessionList'; export { useUser } from './useUser'; export { useClerk } from './useClerk'; +export { useDeepEqualMemo, isDeeplyEqual } from './useDeepEqualMemo'; diff --git a/packages/clerk-js/src/ui/hooks/useDeepEqualMemo.ts b/packages/shared/src/react/hooks/useDeepEqualMemo.ts similarity index 94% rename from packages/clerk-js/src/ui/hooks/useDeepEqualMemo.ts rename to packages/shared/src/react/hooks/useDeepEqualMemo.ts index d86a60666b5..9eb8f0ca7d3 100644 --- a/packages/clerk-js/src/ui/hooks/useDeepEqualMemo.ts +++ b/packages/shared/src/react/hooks/useDeepEqualMemo.ts @@ -16,3 +16,5 @@ const useDeepEqualMemoize = (value: T) => { export const useDeepEqualMemo: UseDeepEqualMemo = (factory, dependencyArray) => { return React.useMemo(factory, useDeepEqualMemoize(dependencyArray)); }; + +export const isDeeplyEqual = deepEqual; From 8cc2a5c47b153fd6cb34f6af5da40b590b68cba0 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Tue, 9 Jan 2024 04:34:45 +0200 Subject: [PATCH 2/3] Create pink-gifts-retire.md --- .changeset/pink-gifts-retire.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/pink-gifts-retire.md diff --git a/.changeset/pink-gifts-retire.md b/.changeset/pink-gifts-retire.md new file mode 100644 index 00000000000..8de046f04c1 --- /dev/null +++ b/.changeset/pink-gifts-retire.md @@ -0,0 +1,7 @@ +--- +"@clerk/clerk-js": patch +"@clerk/clerk-react": patch +"@clerk/shared": minor +--- + +Allow dynamic values components props, even if these values change after the components are rendered. For example, a `SignIn` component with a `redirectUrl` prop passed in will always respect the latest value of `redirectUrl`. From b1913131b26a32915b82d1225492f7e178d31497 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Thu, 11 Jan 2024 12:36:22 +0200 Subject: [PATCH 3/3] fix(clerk-react,shared): Do not equally compare children and customPages React `children` can hold circular references with other `children` arrays and this crashes the deepEqual mechanism The customPages implementation requires the prop references to always change so there is no need to compare these on every prop update --- packages/react/src/components/uiComponents.tsx | 13 ++++++++++--- packages/shared/src/index.ts | 1 + packages/shared/src/object.ts | 7 +++++++ 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 packages/shared/src/object.ts diff --git a/packages/react/src/components/uiComponents.tsx b/packages/react/src/components/uiComponents.tsx index 67cc6b5f6b0..d3f1f324fd8 100644 --- a/packages/react/src/components/uiComponents.tsx +++ b/packages/react/src/components/uiComponents.tsx @@ -1,4 +1,4 @@ -import { logErrorInDevMode } from '@clerk/shared'; +import { logErrorInDevMode, without } from '@clerk/shared'; import { isDeeplyEqual } from '@clerk/shared/react'; import type { CreateOrganizationProps, @@ -90,8 +90,15 @@ type OrganizationSwitcherPropsWithoutCustomPages = Without { private portalRef = React.createRef(); - componentDidUpdate(prevProps: Readonly) { - if (!isDeeplyEqual(prevProps.props, this.props.props)) { + componentDidUpdate(_prevProps: Readonly) { + // Remove children and customPages from props before comparing + // children might hold circular references which deepEqual can't handle + // and the implementation of customPages relies on props getting new references + const prevProps = without(_prevProps.props, 'customPages', 'children'); + const newProps = without(this.props.props, 'customPages', 'children'); + // instead, we simply use the length of customPages to determine if it changed or not + const customPagesChanged = prevProps.customPages?.length !== newProps.customPages?.length; + if (!isDeeplyEqual(prevProps, newProps) || customPagesChanged) { this.props.updateProps({ node: this.portalRef.current, props: this.props.props }); } } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index e1cfa055262..0a148aa7903 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -29,5 +29,6 @@ export * from './poller'; export * from './proxy'; export * from './underscore'; export * from './url'; +export * from './object'; export { createWorkerTimers } from './workerTimers'; export { DEV_BROWSER_JWT_KEY, getDevBrowserJWTFromURL, setDevBrowserJWTInURL } from './devBrowser'; diff --git a/packages/shared/src/object.ts b/packages/shared/src/object.ts new file mode 100644 index 00000000000..e47d399f200 --- /dev/null +++ b/packages/shared/src/object.ts @@ -0,0 +1,7 @@ +export const without = (obj: T, ...props: P[]): Omit => { + const copy = { ...obj }; + for (const prop of props) { + delete copy[prop]; + } + return copy; +};