Skip to content

fix(shared, nextjs): Support importing @clerk/shared in @clerk/backend#1769

Merged
dimkl merged 5 commits intomainfrom
fix-shared-deps-in-backend
Sep 28, 2023
Merged

fix(shared, nextjs): Support importing @clerk/shared in @clerk/backend#1769
dimkl merged 5 commits intomainfrom
fix-shared-deps-in-backend

Conversation

@dimkl
Copy link
Copy Markdown
Contributor

@dimkl dimkl commented Sep 22, 2023 •
edited
Loading

Description

Review it per commit

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

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-extension
  • gatsby-plugin-clerk
  • build/tooling/chore

@dimkl dimkl requested a review from a team as a code owner September 22, 2023 14:16
@dimkl dimkl self-assigned this Sep 22, 2023
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Sep 22, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 8f52000

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@clerk/shared Minor
@clerk/backend Minor
@clerk/clerk-js Patch
@clerk/fastify Patch
@clerk/clerk-react Patch
@clerk/remix Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/clerk-sdk-node Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

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

@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from df920f3 to b90c204 Compare September 22, 2023 14:16
@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from b90c204 to 20d7932 Compare September 22, 2023 16:12
anagstef
anagstef approved these changes Sep 25, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@anagstef anagstef left a comment

Choose a reason for hiding this comment

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

💯

Comment thread packages/shared/src/utils/callWithRetry.ts Outdated
Comment thread packages/backend/src/redirections.test.ts
Comment thread packages/backend/src/redirections.test.ts
@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from 20d7932 to 5d6c90b Compare September 25, 2023 09:20
dimkl
dimkl commented Sep 25, 2023
View reviewed changes
Comment thread packages/backend/src/util/shared.ts

import { buildErrorThrower } from '@clerk/shared';
// TODO: replace packageName with `${PACKAGE_NAME}@${PACKAGE_VERSION}` from tsup.config.ts
export const errorThrower = buildErrorThrower({ packageName: '@clerk/backend' });
Copy link
Copy Markdown
Contributor Author

@dimkl dimkl Sep 25, 2023

Choose a reason for hiding this comment

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

I will fix this in another PR that will update how we build and run tests in backend package (currently tsup is not used in tests)

dimkl
dimkl commented Sep 25, 2023
View reviewed changes
Comment thread packages/shared/src/utils/keys.ts

export function createDevOrStagingUrlCache() {
// TODO: Check if we can merge it with `./instance.ts#isStaging()`
const DEV_OR_STAGING_SUFFIXES = [
Copy link
Copy Markdown
Contributor Author

@dimkl dimkl Sep 25, 2023

Choose a reason for hiding this comment

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

I will cater this in another iteration where we will evaluate the exports of the @clerk/shared

@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch 2 times, most recently from 8fef32a to f43b75f Compare September 25, 2023 16:44
brkalow
brkalow reviewed Sep 25, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@brkalow brkalow left a comment

Choose a reason for hiding this comment

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

Can we add JSDoc comments to the exported methods moved to @clerk/shared as part of this? It should be quick, and leaves it better for the next person to touch this code 🙏

Comment thread packages/shared/src/utils/url.ts
Comment thread packages/shared/src/utils/url.ts
Comment thread packages/shared/src/utils/callWithRetry.ts
@Nikpolik Nikpolik mentioned this pull request Sep 27, 2023
Merged
24 tasks
@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from 6e815d1 to 2d79433 Compare September 27, 2023 12:59
@dimkl
Copy link
Copy Markdown
Contributor Author

dimkl commented Sep 27, 2023

@brkalow JSDocs added to the moved helpers. ptal

@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from 2d79433 to f18819e Compare September 27, 2023 13:58
@dimkl dimkl requested a review from brkalow September 27, 2023 13:58
@dimkl
Copy link
Copy Markdown
Contributor Author

dimkl commented Sep 27, 2023

Since @clerk/nextjs depends on the @clerk/backend we should first merge this PR #1767 and then proceed with this. Otherwise we get the following error when building nextjs:
Screenshot 2023-09-27 at 17 22 49

@dimkl dimkl force-pushed the fix-shared-deps-in-backend branch from f18819e to 8f52000 Compare September 28, 2023 12:10
@dimkl dimkl enabled auto-merge September 28, 2023 12:13
@dimkl dimkl added this pull request to the merge queue Sep 28, 2023
Merged via the queue into main with commit 6dbaa2f Sep 28, 2023
@dimkl dimkl deleted the fix-shared-deps-in-backend branch September 28, 2023 12:26
@clerk-cookie clerk-cookie mentioned this pull request Sep 28, 2023
Merged
@clerk-cookie
Copy link
Copy Markdown
Collaborator

clerk-cookie commented Sep 28, 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.

@clerk clerk locked as resolved and limited conversation to collaborators Sep 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@anagstef anagstef anagstef approved these changes

@brkalow brkalow brkalow approved these changes

Assignees

@dimkl dimkl

Labels

backend

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dimkl @clerk-cookie @brkalow @anagstef