feat(elements): Infer base path - #3557
Conversation
🦋 Changeset detectedLatest commit: 3567e14 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
| * | ||
| * TODO: re-use usePathnameWithoutCatchAll from the next SDK | ||
| * The base path for your sign-in route. | ||
| * Will be automatically inferred in Next.js. |
There was a problem hiding this comment.
In the future we can adjust this line e.g. with Remix once we support this in other frameworks, too
| exampleMode = false, | ||
| fallback = null, | ||
| path = SIGN_IN_DEFAULT_BASE_PATH, | ||
| routing, | ||
| path: pathProp, | ||
| routing = ROUTING.path, |
There was a problem hiding this comment.
Made the falsy exampleMode and routing props more explicit by setting the default values they'd be inferred to anyways
| @@ -0,0 +1,61 @@ | |||
| import { useRouter } from 'next/compat/router'; | |||
| import { useParams, usePathname } from 'next/navigation'; | |||
There was a problem hiding this comment.
So in packages/nextjs/src/client-boundary/hooks/usePathnameWithoutCatchAll.tsx we require these two hooks since the inline comment there says that they'd error out in pages router.
I found this papertrail:
- Using the pathname ( available through usePathname() hook ) in the /pages router vercel/next.js#62880 (comment)
- The useSearchParams() and useParams() hooks behave inconsistently between app and page router vercel/next.js#54242
- Make useSearchParams and useParams compatible between app and pages router vercel/next.js#55280
- https://github.com/vercel/next.js/blob/4398e348ee724d88282881137311680ee60fb23c/packages/next/navigation-types/compat/navigation.d.ts#L13-L20
So I think we're good with importing it directly since the version that got introduced is below our minimum Next.js version
There was a problem hiding this comment.
hmm, does that mean we can update the one within the nextjs package as well ?
There was a problem hiding this comment.
Yeah, probably. But it works in that setup so "don't change a running system"
| 'react/sign-up/index': 'src/react/sign-up/index.ts', | ||
| }, | ||
| external: ['react', 'react-dom', '@statelyai/inspect'], | ||
| external: ['react', 'react-dom', 'next', '@statelyai/inspect'], |
There was a problem hiding this comment.
While https://tsup.egoist.dev/#excluding-packages explains
but dependencies and peerDependencies in your package.json are always excluded
I guess it doesn't hurt explicitly adding it here if we have react and react-dom here already
4 participants
Description
The
pathprop on the<SignIn.Root>and<SignUp.Root>component is now automatically inferred. Previously, the default values were/sign-inand/sign-up, on other routes you had to explicitly define your route.The new heuristic for determining the path where
<SignIn.Root>and<SignUp.Root>are mounted is:pathpropCLERK_SIGN_IN_URLandCLERK_SIGN_UP_URLenv var/sign-inand/sign-upFixes SDK-1791
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change