fix(nextjs, shared): Restores reverted commits and fixes issue with shared in nextjs by dimkl · Pull Request #1807 · clerk/javascript

dimkl · 2023-09-29T22:54:06Z

Description

Review it per commit

After restoring the reverted commits of #1806 , i found that the error was raised because there was some CSR only rendering files in @clerk/shared that did not have the use client directory set.
The culprits were:

packages/shared/src/hooks/contexts.tsx
packages/shared/src/hooks/createContextAndHook.ts

I tested the fix in the local playground/app-router and it seems that there is no error thrown.

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

Packages affected

changeset-bot · 2023-09-29T22:54:09Z

🦋 Changeset detected

Latest commit: 319d166

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

This PR includes changesets to release 11 packages

Name	Type
@clerk/backend	Patch
@clerk/nextjs	Patch
@clerk/shared	Patch
@clerk/fastify	Patch
gatsby-plugin-clerk	Patch
@clerk/remix	Patch
@clerk/clerk-sdk-node	Patch
@clerk/clerk-js	Patch
@clerk/clerk-react	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

LekoArts · 2023-10-02T06:55:35Z

Since we've seen issues with this PR I think this round we should be more conservative and test more extensively. Basically, we'll need to check our usages of useX hooks and see those files have the use client directive.

We probably should do two things for sure:

Test out a snapshot of this PR in apps other than playground/app-router. I'm not confident that this is a complete site that could show all errors
Try using https://github.com/roginfarrer/eslint-plugin-react-server-components and see if it shows anything helpful (I haven't used it yet, only read its code)

panteliselef

Does it make sense to group all client related stuff with a "client-boundary" folder/file like we do for nextjs ?

dimkl · 2023-10-03T13:52:43Z

Does it make sense to group all client related stuff with a "client-boundary" folder/file like we do for nextjs ?

@panteliselef Let's open a discussion in team-js channel and see how it goes. I think it makes sense.

nikosdouvlis · 2023-10-04T09:58:49Z

!snapshot

clerk-cookie · 2023-10-04T10:04:19Z

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

Package	Version
@clerk/backend	0.30.2-snapshot.bffe9d9
@clerk/chrome-extension	0.4.5-snapshot.bffe9d9
@clerk/clerk-js	4.60.0-snapshot.bffe9d9
@clerk/clerk-expo	0.19.7-snapshot.bffe9d9
@clerk/fastify	0.6.12-snapshot.bffe9d9
gatsby-plugin-clerk	4.4.13-snapshot.bffe9d9
@clerk/localizations	1.26.3-snapshot.bffe9d9
@clerk/nextjs	4.25.2-snapshot.bffe9d9
@clerk/clerk-react	4.26.2-snapshot.bffe9d9
@clerk/remix	3.0.4-snapshot.bffe9d9
@clerk/clerk-sdk-node	4.12.11-snapshot.bffe9d9
@clerk/shared	0.24.2-snapshot.bffe9d9
@clerk/themes	1.7.7-snapshot.bffe9d9
@clerk/types	3.54.0-snapshot.bffe9d9

Tip: use the snippet copy button below to quickly install the required packages.

# @clerk/backend
npm i @clerk/backend@0.30.2-snapshot.bffe9d9

# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.4.5-snapshot.bffe9d9

# @clerk/clerk-js
npm i @clerk/clerk-js@4.60.0-snapshot.bffe9d9

# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.19.7-snapshot.bffe9d9

# @clerk/fastify
npm i @clerk/fastify@0.6.12-snapshot.bffe9d9

# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.4.13-snapshot.bffe9d9

# @clerk/localizations
npm i @clerk/localizations@1.26.3-snapshot.bffe9d9

# @clerk/nextjs
npm i @clerk/nextjs@4.25.2-snapshot.bffe9d9

# @clerk/clerk-react
npm i @clerk/clerk-react@4.26.2-snapshot.bffe9d9

# @clerk/remix
npm i @clerk/remix@3.0.4-snapshot.bffe9d9

# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.12.11-snapshot.bffe9d9

# @clerk/shared
npm i @clerk/shared@0.24.2-snapshot.bffe9d9

# @clerk/themes
npm i @clerk/themes@1.7.7-snapshot.bffe9d9

# @clerk/types
npm i @clerk/types@3.54.0-snapshot.bffe9d9

This reverts commit aae1150.

…red imports"" This reverts commit 01bb8ca.

…d drop unused"" This reverts commit 0df7e2c.

…ackage"" This reverts commit 9626959.

…/nextjs) node_modules/@clerk/shared/dist/cjs/hooks/createContextAndHook.js (43:35) @ createContext createContext only works in Client Components. Add the "use client" directive at the top of the file to use it. Read more: https://nextjs.org/docs/messages/context-in-server-component at createContextAndHook (webpack-internal:///(rsc)/./node_modules/ @clerk/shared/dist/cjs/hooks/createContextAndHook.js:48:38) at eval (webpack-internal:///(rsc)/./node_modules/@clerk/shared/ dist/cjs/hooks/contexts.js:53:110)

Even though we had imports of `@clerk/shared` the dependency was missing from package.json of `@clerk/nextjs` and the `@clerk/shared` defined in `@clerk/clerk-react` package.json was used (that's why no error was raised from the imports).

…ache issue: Error: Failed to execute turbo (Unable to locate Go binary). ERROR Unable to find Go binary. Please report this issue at ... turbo GH: vercel/turborepo#5381

clerk-cookie · 2024-10-05T00:01:26Z

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.

dimkl self-assigned this Sep 29, 2023

dimkl requested a review from a team as a code owner September 29, 2023 22:54

clerk-cookie added nextjs backend playground labels Sep 29, 2023

dimkl force-pushed the fix-nextjs-shared-deps branch 2 times, most recently from 635ad37 to 488bc28 Compare September 30, 2023 09:54

dimkl force-pushed the fix-nextjs-shared-deps branch from b71123b to 07c73ae Compare October 2, 2023 10:03

panteliselef approved these changes Oct 2, 2023

View reviewed changes

dimkl force-pushed the fix-nextjs-shared-deps branch 6 times, most recently from f484783 to 0305080 Compare October 3, 2023 13:00

clerk-cookie removed the playground label Oct 3, 2023

dimkl force-pushed the fix-nextjs-shared-deps branch 2 times, most recently from 82278e5 to 3613794 Compare October 3, 2023 13:31

clerk-cookie removed the nextjs label Oct 3, 2023

dimkl force-pushed the fix-nextjs-shared-deps branch 2 times, most recently from c019805 to e59325c Compare October 3, 2023 13:40

clerk-cookie added the nextjs label Oct 3, 2023

dimkl force-pushed the fix-nextjs-shared-deps branch from 0b7ecb7 to c01b89f Compare October 3, 2023 13:51

dimkl force-pushed the fix-nextjs-shared-deps branch from c01b89f to bffe9d9 Compare October 3, 2023 16:28

dimkl force-pushed the fix-nextjs-shared-deps branch from bffe9d9 to 63f77bd Compare October 4, 2023 14:07

Revert "fix(backend): Revert deprecation warnings"

a20e846

This reverts commit aae1150.

dimkl force-pushed the fix-nextjs-shared-deps branch from 63f77bd to 25e6acd Compare October 5, 2023 10:36

dimkl and others added 7 commits October 5, 2023 15:01

Revert "Revert "chore(backend): Replace shared module with @clerk/sha…

9059631

…red imports"" This reverts commit 01bb8ca.

Revert "Revert "chore(backend): Moved shared module to util/shared an…

f658d9c

…d drop unused"" This reverts commit 0df7e2c.

Revert "Revert "chore(shared): Add JSDoc to helpers moved to shared p…

e383f78

…ackage"" This reverts commit 9626959.

update changeset

cf0cbe8

chore(repo): Upgrade turbo to latest to resolve issue with poisoned c…

319d166

…ache issue: Error: Failed to execute turbo (Unable to locate Go binary). ERROR Unable to find Go binary. Please report this issue at ... turbo GH: vercel/turborepo#5381

dimkl force-pushed the fix-nextjs-shared-deps branch from 9acaa62 to 319d166 Compare October 5, 2023 12:01

dimkl added this pull request to the merge queue Oct 5, 2023

dimkl removed this pull request from the merge queue due to a manual request Oct 5, 2023

dimkl added this pull request to the merge queue Oct 5, 2023

Merged via the queue into main with commit 7d3cbcb Oct 5, 2023

dimkl deleted the fix-nextjs-shared-deps branch October 5, 2023 16:24

clerk-cookie mentioned this pull request Oct 5, 2023

Version Packages #1831

Merged

clerk locked as resolved and limited conversation to collaborators Oct 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(nextjs, shared): Restores reverted commits and fixes issue with shared in nextjs - #1807

fix(nextjs, shared): Restores reverted commits and fixes issue with shared in nextjs#1807
dimkl merged 8 commits into
mainfrom
fix-nextjs-shared-deps

dimkl commented Sep 29, 2023 •

edited

Loading

Uh oh!

changeset-bot Bot commented Sep 29, 2023 •

edited

Loading

Uh oh!

LekoArts commented Oct 2, 2023

Uh oh!

panteliselef left a comment

Uh oh!

dimkl commented Oct 3, 2023 •

edited

Loading

Uh oh!

nikosdouvlis commented Oct 4, 2023

Uh oh!

clerk-cookie commented Oct 4, 2023

Uh oh!

Uh oh!

clerk-cookie commented Oct 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

dimkl commented Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Packages affected

Uh oh!

changeset-bot Bot commented Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

LekoArts commented Oct 2, 2023

Uh oh!

panteliselef left a comment

Choose a reason for hiding this comment

Uh oh!

dimkl commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikosdouvlis commented Oct 4, 2023

Uh oh!

clerk-cookie commented Oct 4, 2023

Uh oh!

Uh oh!

clerk-cookie commented Oct 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dimkl commented Sep 29, 2023 •

edited

Loading

changeset-bot Bot commented Sep 29, 2023 •

edited

Loading

dimkl commented Oct 3, 2023 •

edited

Loading