-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: add use-deprecated-from-deprecated #204
Conversation
🦋 Changeset detectedLatest commit: 30a755c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
// Mixed deprecated and non-deprecated imports | ||
{ | ||
code: `import {Button, Tooltip} from '@primer/react'`, | ||
output: `import {Button, } from '@primer/react' |
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.
Is this ,
after the Button expected? Seems like tests are succeeding so maybe it is expected. If so, are we relying on prettier to remove the comma after the fix?
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.
Yeah, let me know if you have a good way to remove it 😅 I couldn't really figure it out. Definitely relying on prettier for this
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.
Are we thinking to use the canary version of the rule to fix the imports since we are not looking into enabling it in the downstread repo? Or do you have a different path in mind?
Thanks for raising this PR, btw. Looking great!
@broccolinisoup I figured that we could ship the rule but not add it to the recommended config so that we could turn it on in dotcom in our PR and autofix and then turn it off Curious what you think 👀 |
Not adding into the recommended config is a great idea! 🚀 @joshblack |
Closes https://github.com/github/primer/issues/3680
Add a rule that will help us migrate code from
@primer/react
to@primer/react/deprecated
as the middle step in deprecating some of these components. This is not a rule that will be enabled by default. Instead, it will help us migrate code in this middle step of migration.