-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add rule preventing importing certain components from react-native #38
Conversation
@marcaaron it seems there is no puller bear in this repo and Github suggested I assign you. You have some context on this so I did, but let me know if there's a different way I should go about assigning! |
One of our contributors recommended we add I went ahead and added that @marcaaron but can revert if you disagree. |
I like that idea but see we have a few
So it's really confusing which one we should be using... Maybe we can create an issue to:
Or we can leave what you have and try to do that cleanup in another PR / issue. Either way works. |
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.
LGTM - feel free to merge @puneetlath or update to remove the TextInput
if you agree we should make those usages more consistent before getting strict here. I don't have strong feelings on this tho so approving.
It looks like the only files importing react native |
In this issue/PR I have renamed our custom
Text
andButton
components toExpensifyText
andExpensifyButton
. We want contributors to use these custom components rather than using the defaultText
andButton
components fromreact-native
.This rule will throw a lint error when either of those two components is imported from React Native. That looks like the following:
The rule can be suppressed by adding a comment like
// eslint-disable-next-line no-restricted-imports
.I have also updated the Readme to say that the App repo needs to be updated after the ESLint configuration is updated, not just Web and Web-Secure.
And finally, I have a list of follow-up todos once this PR is merged here: https://github.com/Expensify/Expensify/issues/187203