Uh oh!
There was an error while loading. Please reload this page.
Construct urls based on Organization or User <OrganizationSwitcher/> - #1503
Conversation
🦋 Changeset detectedLatest commit: 7017b8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
440127a to
043c566Comparepanteliselef
commented
Jul 21, 2023
!snapshot |
clerk-cookie
commented
Jul 21, 2023
Hey @panteliselef - the snapshot version command generated the following package versions:
Tip: use the snippet copy button below to quickly install the required packages. # @clerk/backend
npm i @clerk/backend@0.26.1-snapshot.043c566# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.3.25-snapshot.043c566# @clerk/clerk-js
npm i @clerk/clerk-js@4.54.2-snapshot.043c566# eslint-config-custom
npm i eslint-config-custom@0.3.0# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.18.16-snapshot.043c566# @clerk/fastify
npm i @clerk/fastify@0.6.3-snapshot.043c566# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.4.3-snapshot.043c566# @clerk/localizations
npm i @clerk/localizations@1.23.3-snapshot.043c566# @clerk/nextjs
npm i @clerk/nextjs@4.23.1-snapshot.043c566# @clerk/clerk-react
npm i @clerk/clerk-react@4.23.2-snapshot.043c566# @clerk/remix
npm i @clerk/remix@2.9.1-snapshot.043c566# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.12.1-snapshot.043c566# @clerk/shared
npm i @clerk/shared@0.20.0# @clerk/themes
npm i @clerk/themes@1.7.5# @clerk/types
npm i @clerk/types@3.48.2-snapshot.043c566 |
panteliselef
commented
Aug 3, 2023
!snapshot |
clerk-cookie
commented
Aug 3, 2023
Hey @panteliselef - the snapshot version command generated the following package versions:
Tip: use the snippet copy button below to quickly install the required packages. # @clerk/backend
npm i @clerk/backend@0.28.0-snapshot.a1792a4# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.3.27-snapshot.a1792a4# @clerk/clerk-js
npm i @clerk/clerk-js@4.56.0-snapshot.a1792a4# eslint-config-custom
npm i eslint-config-custom@0.3.0# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.18.18-snapshot.a1792a4# @clerk/fastify
npm i @clerk/fastify@0.6.4-snapshot.a1792a4# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.4.5-snapshot.a1792a4# @clerk/localizations
npm i @clerk/localizations@1.24.2-snapshot.a1792a4# @clerk/nextjs
npm i @clerk/nextjs@4.23.3-snapshot.a1792a4# @clerk/clerk-react
npm i @clerk/clerk-react@4.23.3-snapshot.a1792a4# @clerk/remix
npm i @clerk/remix@2.9.2-snapshot.a1792a4# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.12.3-snapshot.a1792a4# @clerk/shared
npm i @clerk/shared@0.21.0# @clerk/themes
npm i @clerk/themes@1.7.5# @clerk/types
npm i @clerk/types@3.50.0-snapshot.a1792a4 |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| return urlWithParam; | ||
| }; | ||
| const parse = createDynamicParamParser({ regex: /:(\w+)/ }); |
There was a problem hiding this comment.
❓ Do we have a need for multiple parsers right now? If not, I'd prefer a more explicit/simple approach with a more explicit name for the parse function.
There was a problem hiding this comment.
I do like having the createDynamicParamParser util here. I renamed it to populateParamFromObject
There was a problem hiding this comment.
Okay! Can we move it to its own file and write a couple of unit tests covering it? I see the tests for ClerkUIComponentsContext but lets include a few tests to better document this one :)
| const navigateAfterSwitchOrganization = () => | ||
| ctx.afterSwitchOrganizationUrl ? navigate(ctx.afterSwitchOrganizationUrl) : Promise.resolve(); | ||
| const navigateAfterSwitchOrganization = ({ |
There was a problem hiding this comment.
Are we going to keep the internal name or should we rename this to navigateAfterSelectOrganization?
There was a problem hiding this comment.
Not sure if that would be helpful.
If we split this to 2 function then we are exposing the conditional logic inside of handlePersonalWorkspaceClicked in order to have the same result. If in any case we want to create more than 2 handlePersonalWorkspaceClicked functions in more than 2 components this will result to more duplicate code.
There was a problem hiding this comment.
Sorry If I wasn't clear - I'm just saying that we probably want to rename this to better match the new public API
Uh oh!
There was an error while loading. Please reload this page.
nikosdouvlis
left a comment
There was a problem hiding this comment.
Good job @panteliselef :)
Added an extra comment but approved!
…rganizationSwitcher/> + Introduces dynamicParamParser (+tests) + Add types for functions and string literals in OrganizationSwitcherProps & CreateOrganizationProps + afterSelectPersonalUrl & afterSelectOrganizationUrl
clerk-cookie
commented
Aug 8, 2024
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Type of change
Packages affected
@clerk/clerk-js@clerk/clerk-react@clerk/nextjs@clerk/remix@clerk/types@clerk/themes@clerk/localizations@clerk/clerk-expo@clerk/backend@clerk/clerk-sdk-node@clerk/shared@clerk/fastify@clerk/chrome-extensiongatsby-plugin-clerkbuild/tooling/choreDescription
npm testruns as expected.npm run buildruns as expected.Demo for this PR
You can test the changes of this PR in this app