Skip to content

chore(nextjs): Add deprecation warning for app-beta subpath export - #1829

Merged
dimkl merged 3 commits into
mainfrom
cleanups-nextjs-clerkv5
Oct 6, 2023
Merged

chore(nextjs): Add deprecation warning for app-beta subpath export#1829
dimkl merged 3 commits into
mainfrom
cleanups-nextjs-clerkv5

Conversation

@dimkl

@dimkl dimkl commented Oct 4, 2023

Copy link
Copy Markdown
Contributor

Description

The implementation in app-beta was an attempt to support the NextJS app-router early on. Since the app-router release is stable we will deprecate the @clerk/nextjs/app-beta subpath export and suggest using the stable @clerk/nextjs exports instead.

In this PR the following hacks were applied to add JSDocs and console warnings deprecations:

  • we could not add a single deprecation warning in the app-beta/index.ts because it did not work so deprecated was added in each file
  • Replaced import from @clerk/clerk-react with module to show deprecation warning (see: packages/nextjs/src/app-beta/client/clerk-react.ts)
  • Use import with aliases for re-exported modules of @clerk/clerk-react to add JSDoc deprecations only for the exports from the app-beta

Since the whole folder app-beta will be removed in the next major version, there is no need to worry about those hacks.

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 self-assigned this Oct 4, 2023
@dimkl
dimkl requested a review from a team as a code owner October 4, 2023 22:09
@changeset-bot

changeset-bot Bot commented Oct 4, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 78eb66f

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

This PR includes changesets to release 1 package
Name Type
@clerk/nextjs 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 changed the title chore(nextjs): Drop app-beta chore(nextjs): Add deprecation warning for app-beta subpath export Oct 4, 2023
Comment thread packages/nextjs/src/app-beta/ClerkProvider.tsx Outdated
@dimkl
dimkl force-pushed the cleanups-nextjs-clerkv5 branch from 7fb289e to ebfd84a Compare October 5, 2023 09:54
@dimkl
dimkl force-pushed the fix-nextjs-shared-deps branch from 63f77bd to 25e6acd Compare October 5, 2023 10:36
@dimkl
dimkl force-pushed the cleanups-nextjs-clerkv5 branch from ebfd84a to 77ff3da Compare October 5, 2023 10:40
@dimkl
dimkl force-pushed the fix-nextjs-shared-deps branch from 9acaa62 to 319d166 Compare October 5, 2023 12:01
Base automatically changed from fix-nextjs-shared-deps to main October 5, 2023 16:24
@dimkl
dimkl force-pushed the cleanups-nextjs-clerkv5 branch from 77ff3da to f521c21 Compare October 5, 2023 16:38
@dimkl
dimkl requested a review from LekoArts October 5, 2023 16:38
@dimkl
dimkl enabled auto-merge October 5, 2023 16:38
@brkalow

brkalow commented Oct 5, 2023

Copy link
Copy Markdown
Member

@dimkl What were the issues you ran into with adding the deprecation warning to app-beta/index?

@dimkl

dimkl commented Oct 6, 2023

Copy link
Copy Markdown
Contributor Author

@brkalow , @LekoArts adding a deprecation in the app-beta/index.ts seems to be ignored.
I have created the following screenshots to showcase that add a warning to the module, the warning is visible to the console but the warning in the index.ts is not.
The following steps were applied:

  • add deprecation warning to index.ts and to auth.ts and a console.log
  • <javascript_repo> git status to show that those changes were saved
  • <javascript_repo> npm run build -- --force && npm run yalc:all to create clean build and push that build to the playground
  • <javascript_repo>/playground/nextjs npm run dev to start an application with NextJS app-beta
  • navigate to http://localhost:4011 to trigger application logs
  • Search for deprecation warnings and console log in the application logs
Screenshot 2023-10-06 at 13 04 47 Screenshot 2023-10-06 at 13 28 24 Screenshot 2023-10-06 at 13 29 56

@dimkl
dimkl force-pushed the cleanups-nextjs-clerkv5 branch from 7beee2a to 78eb66f Compare October 6, 2023 12:12
@LekoArts

LekoArts commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Ok, makes sense 👍 I'd need to dive deeper into how you could run a side-effect when a module is imported. With a quick search I couldn't find previous art on this.

@dimkl
dimkl added this pull request to the merge queue Oct 6, 2023
Merged via the queue into main with commit de5e2f5 Oct 6, 2023
@dimkl
dimkl deleted the cleanups-nextjs-clerkv5 branch October 6, 2023 17:31
@clerk-cookie clerk-cookie mentioned this pull request Oct 6, 2023
@clerk-cookie

Copy link
Copy Markdown
Collaborator

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 Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants