Babel plugin to remove `data-testid`

[ematipico]

Describe the feature you'd like:

It would be nice to have a babel plugin that people can use when building production. This babel plugin will remove data-testid from the rendered HTML.

I know it can be done in userland, but it would be nice if this library would provide an official implementation.

About the name of this plugin, I leave it open. At the then it does just one tiny thing.

Suggested implementation:

module.exports = function() {
    return {
        visitor: {
            JSXAttribute(path) {
                if (path.node.name.name === 'data-testid') {
                    path.remove();
                }
            }
        }
    };
};

Describe alternatives you've considered:

Userland implementation.

Teachability, Documentation, Adoption, Migration Strategy:

The users will have to add it to their babel plugin file

{
    "env": {
        "production": {
            "plugins": ["@testing-library/babel-plugin-react"]
        } 
    }
}

[eps1lon]

TL;DR: data-testid should be set in your test file not your component source. If you need it anyway try babel-plugin-react-remove-properties

I don't think your components should have them baked in. It's IMO more of a convenience selector for your tests e.g. <Button data-testid="trigger">Open Menu</Button><ActionMenu items={['copy link', 'quote reply']} open={true} /> for when getByText is too brittle for your liking.

[ematipico]

When you test containers or components that render more and more components, it's not possible to pass the data-testid to the children anymore. So I end up to have data-testid in some parts of the HTML in order to make my queries more effective.

I wasn't aware of the plugin! Thanks! Although I would be nice if this library would provide their own babel plugin because removing the property would work in a different way for Vue/Svelte etc. in case the users are not using jsx.

[kentcdodds]

Hi @ematipico,

That sounds interesting. If you want to work on it that's great, but it's not going to happen in this repo, so I'll close the issue. Once you have a project ready to go, then feel free to comment in this issue!

[kentcdodds]

I should also note that I think it's unnecessary to remove data-testid attributes for two reasons:

1. You shouldn't have to use the *ByTestId queries very often, so you shouldn't have many data-testid attributes anyway.
2. You should probably run some of your E2E tests in production or a production-like environment and in that case you'll need those data-testid attributes.
3. They add such a small and insignificant amount of weight to your app that you'd be way better served to work on something else to improve the performance of your app :)

[htulipe]

I don't agree with data-testid being ok to stay in production code. It's not a matter of how big it will weigh or how many you have but more a matter of "code cleaness". The definition of "cleaness" is really subjective though.

Anyhow here is how I solved the issue if it can help anyone. I'm using a helper function:

const useTestId = (id: string) => {
  if (process.env.NODE_ENV !== 'test') return {};
  return {
    'data-testid': id
  };
};

// In some component
const testId = useTestId('my-test-id');
return <div {...testId}>...</div>

This way, you won't have any data-testid in your prod build. In addition, a code optimizer could simplify your prod build all the way to const testId = {}; which is, IMO, much better than having your DOM "polluted".

[clehene]

This way, you won't have any data-testid in your prod build. In addition, a code optimizer could simplify your prod build all the way to const testId = {}; which is, IMO, much better than having your DOM "polluted".

So it's ok to "pollute" the code that you read all day, but for some reason not ok to "pollute" the code the browser "reads" every now and then, all while losing the ability to test production deployments

[htulipe]

I'm not sure to follow what you mean @clehene, it was just a suggestion, if you need to run test on production build surely this approach won't work. About polluting the code developers read I don't think that the approach adds much overweight compare to a straight "data-test-id" in your JSX. Anyhow there is lot of subjectivity on this matter.

[eps1lon]

They add such a small and insignificant amount of weight to your app that you'd be way better served to work on something else to improve the performance of your app :)

To add some real world examples to this:

• facebook.com (logged in): document.querySelectorAll('[data-testid]').length === 116
• twitter.com (logged in): document.querySelectorAll('[data-testid]').length === 74
• youtube.com (logged in): document.querySelectorAll('[data-testid]').length === 0

[clehene]

I'm not sure to follow what you mean @clehene, it was just a suggestion, if you need to run test on production build surely this approach won't work. About polluting the code developers read I don't think that the approach adds much overweight compare to a straight "data-test-id" in your JSX. Anyhow there is lot of subjectivity on this matter.

It's perhaps subjective. I also believe the data-test-* is wrongly shaped. Here's why: <div test-something=random /> in your code is the actual pollution. The browser doesn't care. What pollutes the DOM and code is having unnecessary structure, the test-id=random adds on that. However

<ul type='apple'/>
 <li id=${apple-id}>..</li>

Basically I have a container (ul) that has a contained type (apple) Is the same as List<Apple> - you don't add anythign but are now type aware (and semantic) There's no reason why you would obfuscate this in production. Of course you can do the same with just divs and. The bottom line, is fundamentally what happens is you want to retain type information at runtime. If done right a plugin like the one suggested would perform a type erasure.

This is perhaps the reason why you don't see this stuff on youtube.com as @eps1lon suggests Let's take a look:

I don't think using id for everything is best either. If you want to keep things simple you have to separate type and identity. So perhaps if were to get back to data-* we should have data-type, data-id as orthogonal concerns.

[abhivaikar]

Curious to know from folks like @clehene / @kentcdodds / @htulipe / @eps1lon in this thread - have you had to think about potential data scraping issue (security perspective) if you leave data-testids in production build?

How strong of an argument this is according to you to strip off the data-testids on prod?

[firmanserdana]

Curious to know from folks like @clehene / @kentcdodds / @htulipe / @eps1lon in this thread - have you had to think about potential data scraping issue (security perspective) if you leave data-testids in production build?

How strong of an argument this is according to you to strip off the data-testids on prod?

It's an old issue, but I am thinking to obfuscate or remove those test-ids so that it wont be easily abused