-
Notifications
You must be signed in to change notification settings - Fork 253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(shared): Add Clerk prefix in deprecation warnings #1890
Conversation
🦋 Changeset detectedLatest commit: 113c14d The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
@@ -29,7 +29,7 @@ export const deprecated = (fnName: string, warning: string, key?: string): void | |||
displayedWarnings.add(messageId); | |||
|
|||
console.warn( | |||
`DEPRECATION WARNING: "${fnName}" is deprecated and will be removed in the next major release.\n${warning}`, | |||
`DEPRECATION WARNING: Clerk "${fnName}" is deprecated and will be removed in the next major release.\n${warning}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this or similar?
`DEPRECATION WARNING: Clerk "${fnName}" is deprecated and will be removed in the next major release.\n${warning}`, | |
`Clerk - DEPRECATION WARNING: "${fnName}" is deprecated and will be removed in the next major release.\n${warning}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we'd want we could get fancy and use something like https://github.com/GoogleChrome/workbox/blob/v7/packages/workbox-core/src/_private/logger.ts
This relates to https://developer.mozilla.org/en-US/docs/Web/API/console#styling_console_output
So we could do:
console.warn('%cClerk', styles.join(';'), 'MESSAGE')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the styles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's open a discussion in Slack about theming our logs in general and implement it as different PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikosdouvlis i have updated the formatting of the warnings. PTAL
31c6904
to
113c14d
Compare
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. |
Description
Add
Clerk
prefix in deprecation warningsChecklist
npm test
runs as expected.npm run build
runs as expected.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-extension
gatsby-plugin-clerk
build/tooling/chore