Rename and clean up custom Text and Button components.

[roryabraham]

Problem

I have observed in my own work and in PR reviews that a common pitfall in Expensify.cash is to do this:

import {Text} from 'react-native';

Which will result in text that does not have the correct font, line height, etc...

Why this is important?

Especially for new contributors, or when working with fancy IDE's such as WebStorm that auto-import things for you, it's an easy mistake to make. It's also an easy mistake to miss, because it's not always easy to tell the difference between GT America and the default font (on Android, in particular).

Solution

Rename our custom Text component and our custom Button component to ExpensifyText and ExpensifyButton, respectively. That should help make it clear to newcomers and reviewers that we should generally always be using our custom component. Some potential bonus solutions:

1. Put something like this in index.js:

   import {AppRegistry, Text} from 'react-native';
   
   ...
   
   if (__DEV__) {
       Text.render = () => {
           throw new Error('Must use <ExpensifyText /> instead of <Text />');
       };
   }

2. Implement a new ESLint rule that complains if you create a custom component that shadows the name of a standard RN library component.

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.0.53-0

[marcaaron]

Really like the idea to clean these up. I recently went through and did something similar with all the TouchableOpacity so that they would use a Button component instead.

I like the idea to try to prevent incorrect usages, but maybe you're right that the name change is enough for now. Probably once there are no more usages of Text people will just naturally notice and encourage use of something like ExpensifyText.

[roryabraham]

Removing N6 hold, might work on this from the plane later today

[roryabraham]

Going to send this back into the pool.

[puneetlath]

I've renamed the components in #6538. I'll announce it in #expensify-open-source once it's merged so people know as well.

There are still some cases of the react-native TouchableOpacity, Pressable, Text, and Button components being used. I didn't touch those. I just renamed the cases where our custom components were used.

I'll look into the bonus solutions a bit as well.

Put something like this in index.js:

I don't think this will work because our custom components and some other ones do use the react-native Text component.

[roryabraham]

Nice! A little while back, @marcaaron wrote up this handy-dandy SO on writing custom ESLint plugins. So we could make a lint rule prohibiting the use of the RN default components, and just disable the rule in the locations where we're intentionally making exceptions. Still a bonus solution, but it's doable if it interests you 😄

[puneetlath]

Nice, thanks for that!

I looked into this a bit and I think there's actually an easier way we can do it rather than creating a custom rule. ESLint has a default rule called no-restricted-imports which allows you to restrict certain imports from certain modules. So we can just restrict the use of Text, Button, and TouchableOpacity from react-native with a rule here like this:

'no-restricted-imports': ['error', {
    'paths': [{
        'name': 'react-native',
        'importNames': ['Text'],
        'message': 'Please use our custom ExpensifyText component instead.',
    }]
}]

It seems that you can't have RuleTester test their built-in rules, but it seems to work well:

What do you think @roryabraham @marcaaron?

[marcaaron]

Ooh nice find @puneetlath! Seems perfect for this use case.

[puneetlath]

Sweet, I'll get that added. Tracking the todos for it here: https://github.com/Expensify/Expensify/issues/187203

[puneetlath]

@anthony-hull suggested we also add an ESLint rule for TextInput since we have a handful of our own custom Text Input components as well: ExpensiTextInput, TextInputAutoWidth, TextInputFocusable, TextInputWithFocusStyles, TextInputWithLabel.

I think it makes sense. What do y'all think?

[roryabraham]

Sure, sounds good to me @puneetlath