Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(types,clerk-react): Make routerPush / routerReplace both required or both optional #2227

Merged
merged 6 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tiny-forks-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-react': minor
'@clerk/types': minor
---

Update the TypeScript types of `<ClerkProvider />`. If you use the `routerPush` prop you're now required to also provide the `routerReplace` prop (or other way around). You can also not provide them at all since both props are optional.
8 changes: 7 additions & 1 deletion .github/workflows/ui-retheme-changes-reminder.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ on:
- main
paths:
- 'packages/clerk-js/src/ui/**'

# files with matching `*.retheme.ts` retheme variant
- 'packages/localizations/src/en-US.ts'
- 'packages/localizations/src/index.ts'
- 'packages/types/src/appearance.ts'
- 'packages/types/src/clerk.ts'
- 'packages/types/src/index.ts'
- 'packages/types/src/localization.ts'
Comment on lines +9 to +15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

jobs:
check-changes:
runs-on: ubuntu-latest
Expand Down
4 changes: 3 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ dist
examples
node_modules
package-lock.json
playground
playground
packages/backend/tests/**/*.js
/**/CHANGELOG.md
19 changes: 19 additions & 0 deletions packages/react/src/contexts/__tests__/ClerkProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,23 @@ describe('ClerkProvider', () => {
expectTypeOf({ publishableKey: 'test' }).not.toMatchTypeOf<ClerkProviderProps>();
});
});

describe('navigation options', () => {
it('expects both routerPush & routerReplace to pass', () => {
expectTypeOf({
publishableKey: 'test',
children: '',
routerPush: () => {},
routerReplace: () => {},
}).toMatchTypeOf<ClerkProviderProps>();
});

it('errors if one of routerPush / routerReplace is passed', () => {
expectTypeOf({
publishableKey: 'test',
children: '',
routerPush: () => {},
}).not.toMatchTypeOf<ClerkProviderProps>();
});
});
});
20 changes: 6 additions & 14 deletions packages/react/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
SignInRedirectOptions,
SignUpRedirectOptions,
UserResource,
Without,
} from '@clerk/types';
import type React from 'react';

Expand All @@ -23,7 +24,7 @@ declare global {
}
}

export type IsomorphicClerkOptions = Omit<ClerkOptions, 'isSatellite'> & {
export type IsomorphicClerkOptions = Without<ClerkOptions, 'isSatellite'> & {
Clerk?: ClerkProp;
clerkJSUrl?: string;
clerkJSVariant?: 'headless' | '';
Expand All @@ -37,19 +38,10 @@ export type ClerkProviderProps = IsomorphicClerkOptions & {
initialState?: InitialState;
};

// TODO(@dimkl): replacing it with the following make nextjs type tests fail
// `Exclude<IsomorphicClerkOptions, 'publishableKey'> & { publishableKey?: string }`
// find another way to reduce the duplication.
export type ClerkProviderOptionsWrapper = Omit<ClerkOptions, 'isSatellite'> & {
Clerk?: ClerkProp;
clerkJSUrl?: string;
clerkJSVariant?: 'headless' | '';
clerkJSVersion?: string;
sdkMetadata?: SDKMetadata;
export type ClerkProviderOptionsWrapper = Without<IsomorphicClerkOptions, 'publishableKey'> & {
publishableKey?: string;
} & MultiDomainAndOrProxy & {
children: React.ReactNode;
};
children: React.ReactNode;
};

export interface BrowserClerkConstructor {
new (publishableKey: string, options?: DomainOrProxyUrl): BrowserClerk;
Expand All @@ -75,7 +67,7 @@ export interface MountProps {
}

export interface HeadlessBrowserClerk extends Clerk {
load: (opts?: Omit<ClerkOptions, 'isSatellite'>) => Promise<void>;
load: (opts?: Without<ClerkOptions, 'isSatellite'>) => Promise<void>;
updateClient: (client: ClientResource) => void;
}

Expand Down
22 changes: 20 additions & 2 deletions packages/types/src/clerk.retheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,25 @@ export type CustomNavigation = (to: string, options?: NavigateOptions) => Promis

export type ClerkThemeOptions = DeepSnakeToCamel<DeepPartial<DisplayThemeJSON>>;

export interface ClerkOptions {
/**
* Navigation options used to replace or push history changes.
* Both `routerPush` & `routerReplace` OR none options should be passed.
*/
type ClerkOptionsNavigationFn =
| {
routerPush?: never;
routerReplace?: never;
}
| {
routerPush: (to: string) => Promise<unknown> | unknown;
routerReplace: (to: string) => Promise<unknown> | unknown;
};

type ClerkOptionsNavigation = ClerkOptionsNavigationFn & {
routerDebug?: boolean;
};

export type ClerkOptions = ClerkOptionsNavigation & {
appearance?: Appearance;
localization?: LocalizationResource;
/**
Expand Down Expand Up @@ -535,7 +553,7 @@ export interface ClerkOptions {
};

sdkMetadata?: SDKMetadata;
}
};

export interface NavigateOptions {
replace?: boolean;
Expand Down
29 changes: 20 additions & 9 deletions packages/types/src/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -495,16 +495,27 @@ export type CustomNavigation = (to: string, options?: NavigateOptions) => Promis

export type ClerkThemeOptions = DeepSnakeToCamel<DeepPartial<DisplayThemeJSON>>;

export interface ClerkOptions {
appearance?: Appearance;
localization?: LocalizationResource;
/**
* Navigation
*/
routerPush?: (to: string) => Promise<unknown> | unknown;
routerReplace?: (to: string) => Promise<unknown> | unknown;
/**
* Navigation options used to replace or push history changes.
* Both `routerPush` & `routerReplace` OR none options should be passed.
*/
type ClerkOptionsNavigationFn =
| {
routerPush?: never;
routerReplace?: never;
}
| {
routerPush: (to: string) => Promise<unknown> | unknown;
routerReplace: (to: string) => Promise<unknown> | unknown;
};

type ClerkOptionsNavigation = ClerkOptionsNavigationFn & {
routerDebug?: boolean;
};

export type ClerkOptions = ClerkOptionsNavigation & {
appearance?: Appearance;
localization?: LocalizationResource;
polling?: boolean;
selectInitialSession?: (client: ClientResource) => ActiveSessionResource | null;
/** Controls if ClerkJS will load with the standard browser setup using Clerk cookies */
Expand Down Expand Up @@ -536,7 +547,7 @@ export interface ClerkOptions {
};

sdkMetadata?: SDKMetadata;
}
};

export interface NavigateOptions {
replace?: boolean;
Expand Down
Loading