Skip to content
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

Consider adding babel-plugin-jsx-remove-data-test-id when react is true #49

Open
satazor opened this issue Jan 23, 2020 · 5 comments
Open

Comments

@satazor
Copy link
Contributor

satazor commented Jan 23, 2020

We are using data-test-id attributes as recommended by some testing libraries, such as RTL. However, ideally these attributes should not "leak" to production.

Consider adding https://www.npmjs.com/package/babel-plugin-jsx-remove-data-test-id if NODE_ENV !== 'test'. One reason to not remove them is if we want to perform e2e testing. We should reflect on this first.

@AfonsoVReis
Copy link
Contributor

We should hide this attributes by default for all node env except test by default , but also provide an option to disable it in cases where e2e testing is used.

Also the babel-plugin-jsx-remove-data-test-id allow us to hide additional custom attribute names which can be useful, should we consider this as well ?

@satazor

@acostalima
Copy link

acostalima commented Feb 25, 2020

Should consider creating a new environment later on, e.g. 'test-e2e', to create builds targeted for e2e tests? If we go down this road, we have the option to decide whether to create an optimized build or not.

@jgradim
Copy link

jgradim commented May 14, 2020

We should definitely add this to our babel-preset.

@threequartersjohn
Copy link

Ooops, sorry 🙏

I was about to mention that we could instead use babel-plugin-react-remove-properties, which is less for specific attributes and would cover both this need for data-test-id and @AfonsoVReis 's suggestion either now or at some point going forward, should the need arise.

@ubmit
Copy link

ubmit commented May 15, 2020

Ooops, sorry 🙏

I was about to mention that we could instead use babel-plugin-react-remove-properties, which is less for specific attributes and would cover both this need for data-test-id and @AfonsoVReis 's suggestion either now or at some point going forward, should the need arise.

I think this is the best approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants