feat(no-debug): render function names

[benmonro]

we commonly wrap render with redux and other things, so having the ability to configure the render function names will be useful for us.

[benmonro]

@Belco90 can you review? we would love to add this rule to our config, but need to be able to configure the render function names.

[thomaslombart]

Hi @benmonro, thanks for your contribution!

You're right, we should support the case where one uses custom render functions. However, I feel it's frustrating for the user to manually set up the different custom render functions it may use in the plugin. It shouldn't have to do this.

If you need it right now, you can rename your imports (import { renderWithRedux as render }) or maintain a fork of this plugin.

@Belco90 What's your take on this? Should we trigger the warning on every debug statement or should we provide options as proposed in this PR? If we go with the first solution, it may frustrate users who uses the debug package as it will trigger false positives.

[benmonro]

We shouldn't have to rename for this. Also sometimes we use both. The user only had to setup the custom functions if they don't want the default

[thomaslombart]

That's why I said "right now" 🙂, it's just temporary.

[benmonro]

Lol ok yeah that would actually require a lot of changes and might even cause potential issues, so wouldn't even work as a temporary solution.

I am curious why you feel it's frustrating for users to have to configure the custom functions? It's a pretty standard practice in lint rules. Take the quotes rule for example. It would usually be done just once in the lint config

[Belco90]

Hey @benmonro first of all thanks for your PR! Obviously we didn't include many config options in v1 so the plugin is still pending to include more options and autofixable code for some rules, and new rules of course!

About this one in particular, it completely makes sense at all to be able to config which render you want to use (and maybe we could even need the same in other rules in the future, we will see). @thomlom Looking at the implementation of this PR the custom renders arg is optional so I don't see why this could bother users as they don't have to setup anything for basic use (i.e. just analyze 'render'). If users want to check custom render, I'm afraid we can only give that option through rule args as we can't guess which custom render names users would use.

I'm happy with the implementation here unless there is some other issue I'm missing, so I'm gonna wait for @thomlom to clarify his thoughts about this.

[thomaslombart]

In my opinion, it can be easy to forget to update this rule's options. If you use for example renderWithRedux and rename it later renderRedux, one can easily forget to update the rule and thus the rule wouldn't throw anymore. I'm also wondering what happens if you have a lot of custom render functions. It seems tedious to write all your possible custom render functions in the options and to maintain it. In the end, I would expect the rule to "just work" if I use debug (but then there's the issue of throwing a warning even when you use debug that doesn't come from @testing-library).

But as you said @Belco90, there's no other options right now apart configuring the render functions manually and since the user doesn't have to do anything initially, it's not a big deal 🙂

[benmonro]

@thomlom fair point about the rename but I don't know if another way. If you look at eslint-plugin-jest they have a similar config option for expect-expect

[thomaslombart]

True! LGTM anyway 🙂

I'll let @Belco90 merge it. Thank you for your contribution!

[Belco90]

LGTM too, I'll merge it soon.

[Belco90]

🎉 This PR is included in version 1.1.1

[benmonro]

Thanks all!

[Belco90]

@all-contributors please add @benmonro for code, docs and test

[allcontributors]

@Belco90

I've put up a pull request to add @benmonro! 🎉