-
Notifications
You must be signed in to change notification settings - Fork 346
chore(clerk-js,clerk-react,shared): Add hook to assert the presence of ClerkProvider [SDK-1043] #2299
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
Conversation
🦋 Changeset detectedLatest commit: 06115b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Instead of adding another Context eg ClerkProviderAssertionContext
why not use useIsomorphicClerkContext
or useClientContext
and apply the same pattern as other hooks?
const ctx = useClientContext();
assertContextExists(ctx, ClientContext);
The assertContextExists
already exists in the shared package.
@dimkl So, the interesting thing about that example is that, if That should probably be fixed up, TBD. 👍 Happy to use I'll fix up the existing usage mentioned above with this updated approach. |
a6c9785
to
d4267a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of another PR, I think we also need to consolidate those:
export function assertContextExists(contextVal: unknown, providerName: string): asserts contextVal { export function assertContextExists(contextVal: unknown, providerName: string): asserts contextVal {
c9302ae
to
d4267a5
Compare
d4267a5
to
49e6d01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…f ClerkProvider [SDK-1043] (#2299) * chore(clerk-js,shared): Add hook to assert the presence of ClerkProvider * chore(clerk-js,clerk-react,shared): Add changeset * chore(shared): Update messaging * chore(clerk-js,shared): Re-used existing context * chore(shared): Require useAssertWrappedByClerkProvider param * Update .changeset/slow-bugs-exist.md * chore(shared): Update error messaging --------- Co-authored-by: Lennart <[email protected]>
Description
Adds and implements
useAssertWrappedByClerkProvider
in order to assert thatClerkProvider
is present. The goal of this being that we'd like to provide end-users a better development experience.This applies to all public-facing components and hooks.
The intention behind introducing a new context was to limit the amount of refactoring needed to support what we were looking to achieve.
Current error messaging:
AuthContext not found
New error messaging:
@clerk/clerk-react: useAuth must be used within the <ClerkProvider> component. Please see: https://clerk. com/docs/components/clerk-provider
SDK-1043
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/clerk-expo
@clerk/fastify
gatsby-plugin-clerk
@clerk/localizations
@clerk/nextjs
@clerk/clerk-react
@clerk/remix
@clerk/clerk-sdk-node
@clerk/shared
@clerk/themes
@clerk/types
build/tooling/chore