App: [HOLD for payment 2022-09-16] [$500] New Lint Rule: DisplayName property

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Write a functional component but don’t set the displayName property OR write a class component and set the displayName property.

Expected Result:

ESLint should complain.

Actual Result:

ESLint does not complain.

View all open jobs on GitHub

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 40 (33 by maintainers)

Most upvoted comments

I think we can cap the price for this.

parasharrajat on Jul 28, 2022

Happy to help with publishing the package

MonilBhavsar on Aug 15, 2022

Super quick turnaround there, thanks. Paid.

arielgreen on Sep 16, 2022

It seems there will be around 20 - 30 errors in the main repo that need to be fixed when this rule is added, which is a bit more extra work.

@andrewlor They’re just displayName errors right? If yes, I’ll fix them with a follow-up PR because it shouldn’t take more than 10 mins.

rushatgabhane on Aug 12, 2022

Looks good to me too 👍 Since this property displays error message according to class/functional component, I’m fine with single rule.

MonilBhavsar on Aug 12, 2022

Ohh, so filename is inferred from the foldername. That should work. If there are any edge cases, the tests will catch them and we can fix them in the PR

rushatgabhane on Aug 8, 2022

Good point. We can just modify slightly to handle that case.

            // Infer the component name from the filename
            const filenameParts = _.split(context.getFilename(), '/');
            let componentName = _(filenameParts).last().split('.')[0];
            if (componentName === 'index') {
                componentName = filenameParts[filenameParts.length - 2];
            }

Hopefully that should cover everything? Assuming component files should only be in those two formats: Test/index.js or Test.js.

andrewlor on Aug 8, 2022

@andrewlor you’ll be creating a new file over here https://github.com/Expensify/eslint-config-expensify/tree/main/eslint-plugin-expensify right?

@rushatgabhane Correct. As I understand it, for each rule I should add two files: eslint-plugin-expensify/<rulename>.js and eslint-plugin-expensify/tests/<rulename>.test.js, and also add an entry in https://github.com/Expensify/eslint-config-expensify/blob/main/rules/expensify.js.

As for your second point, having these as two separate rules, I think that makes sense could make the code slightly more simple to read. I have no objections writing it like that.

andrewlor on Aug 6, 2022

@andrewlor you’ll be creating a new file over here https://github.com/Expensify/eslint-config-expensify/tree/main/eslint-plugin-expensify right?

rushatgabhane on Aug 6, 2022