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

How to deal with dynamic class names à la CSS Modules or Styled Components? #1212

Closed
chapati23 opened this issue Jan 21, 2018 · 28 comments
Closed

Comments

@chapati23
Copy link

chapati23 commented Jan 21, 2018

Any pointers on how to deal with dynamic class names produced by e.g. CSS Modules or styled-components?

The easy way out would be to just plaster my markup with id="e2e-blabla" but that doesn't feel particularly nice to be honest.

Another way could be to look for partial matches in class names. For example the CSS Modules class .form produces the actual class name .styles__form--3gcRC in the DOM. One could now use a CSS attr selector such as [class*=form] which would work in many case but can lead to name conflicts and kinda removed all the benefits of having locally scoped and unique class names.


Current behavior:

There's no documented way (at least i couldn't find one in neither issues nor docs) to make cypress play well with dynamic class names generated by commonly used tools such as CSS Modules or styled-components

Desired behavior:

There's a well documented way of having cypress be able to look for dynamic class names in the DOM

How to reproduce:

Try to make cypress work with any React app that uses either CSS Modules or styled-components

  • Operating System: macOS 10.13.2
  • Cypress Version: 1.4.1
  • Browser Version: Chrome 63.0.3239.132 (Official Build) (64-bit)
@andreiashu
Copy link

Interested as well in this subject. At the moment we're using a mix of unique class names (ie. "payment-button") with [class*=form] type selectors.

@bahmutov
Copy link
Contributor

how would you prefer doing this? I used [class*=form] and [class^=form], anything beyond this would be framework-specific

@jennifer-shehane
Copy link
Member

Wanted to say, @willklein mentioned this in his talk, basically he recommends adding classes purely for testing environment and then stripping them out for production. Perhaps he can elaborate more on how they did that for their Cypress tests.

Link to video here: https://youtu.be/rICGz5qrYJU?t=1h42m28s

@chapati23
Copy link
Author

chapati23 commented Jan 22, 2018

yeah, one could always come up with a conventional prefix such as className="cypress-email-input" or className="e2e-email-input" which would be relatively easy to instrument away.
but then i couldn't run e2e tests against staging or prod environments. And it's also an additional step that has to be remembered while coding. The perfect solution would be to just use the same way of addressing elements that I'm already using in my React components.

Forgive me the question, didn't have time to read too deep into the underlying implementation of cypress yet, but I'd assume you're using webpack?

If that's the case, you could offer people to provide their own loaders. In that case I could just point cypress to the same webpack loaders i'm already using for my dev server (e.g. postcss-loader).

For example, in my cypress test i could then just do the following:

/* src/components/email-form/styles.css */

/*
 * css modules will transform this class
 * into sth like this: .styles__email-input-cmXvUE
 */
.email-input {
  color: blue;
}
/* cypress/integration/email-form.spec.js */

// path is a bit ugly but could be mitigated by using NODE_PATH
import styles from '../../src/components/email-form/styles.css'
const URL = 'http://localhost:8080'

describe('Email Form', () => {
  before(() => {
    cy.visit(`${URL}/`)
  })

  it("should have an email input", () => {
    cy
      // .get("[class*=email-input)") => the current way of doing things
      // the same code i'm using to style my React components which would evaluate to the same .styles__email-input-cmXvUE
      .get(styles.emailInput) // => the new, more exact, way of doing things
      .should('have.attr', 'type', 'email')
  })
})

@zth
Copy link

zth commented Jan 25, 2018

We use the "data-testid"-convention inspired by BEM-naming, which I think works great for us. Example:

<div class="some-class" data-testid="Login__form__container">
  ...
</div>

And then we select test elements using a custom Cypress command that basically just wraps cy.get:

cy.getTestElement('Login__form__container')

// getTestElement, just the function here, but it's a Cypress command in our codebase
function getTestElement(selector) {
  return cy.get(`[data-testid="${selector}"]`);
}

The main benefit for us is that we have a clear convention for how to name test stuff that's not coupled to actual classes or similar. This means that we can refactor stuff and keep our tests as long as we assign the appropriate test id:s to the right elements again when refactoring.

It also allows us to write tooling around that convention (a Chrome extension that lists all available testid:s on a page, with click-to-copy-selector etc), but that type of functionality can of course be implemented regardless of what naming convention you use.

Anyway, just a thought!

@willklein
Copy link

Sorry, I just noticed this. @zth describes the technique I prefer: custom data attributes. Using data-* attributes is framework-agnostic and I think the most balanced approach. His reasoning is spot on, too, I like that it decouples your tests from any style-oriented classes. I still like this approach even if you have semantic class-based selectors, though I wouldn't say you need to introduce it unless you run into actual problems.

We didn't need to BEM name things, but that's also a pretty good idea. In our case, we put semantic descriptors in our form element containers along with data model specific descriptors because we had them. We had a lot of shared, pure components and with only modifying about five lines of template code, we instrumented an infinite number of fields, buttons, and links. Instrumenting other apps may require more changes, but it should still be worthwhile.

How you configure your build to remove this for production will get into framework-specific tooling. If you're using React and Babel, this is what I use:

https://github.com/oliviertassinari/babel-plugin-react-remove-properties.

data-test is the default property but it can be configured if you want something different, or to use more than one.

If there's any desire from the Cypress team, I'm happy to try writing up a guide on this if it fits in the documentation.

@brian-mann
Copy link
Member

We are all for data-test or data-cy.

In fact we've talked about making this the default attribute we use for the Selector Playground - which would make it super nice to automatically find these.

@krishnaxv
Copy link

@brian-mann @willklein Thumbs up 👍 for data-test or data-cy approach.

Any input on this approach for a React project with Create React App (CRA) and styled-components without ejecting?

Create React App does not use .babelrc specified in the project. In webpack.config.prod.js of CRA, babelrc is set to false. I tried react-scripts@2.0.0-next.47d2d941 also, but again unable to use this approach without ejecting CRA.

Any thoughts?

@willklein
Copy link

@krishnaxv that seems like a clear trade off when using something like Create React App: you're giving up control/configuration to have things managed for you. I definitely try to keep from ejecting whenever possible, but if I'm starting to spin up a lot of tooling and need my own Babel plugins (I often do), I find it's time to eject.

@pgroot91
Copy link

pgroot91 commented Mar 2, 2018

@andreiashu @bahmutov do you guys know already if there is a solution for those use cases? Maybe i missed something but didn't saw anyone mentioning your questions with a solution or any workaround or this is something on the map to be added in near future in cypress maybe?

@bahmutov
Copy link
Contributor

bahmutov commented Mar 2, 2018

We strongly recommend adding custom attributes for testing. Especially if they follow the convention, then they become even compatible with CSS Selector Playground. See these two resources:

@bahmutov bahmutov closed this as completed Mar 2, 2018
@andrewdang17
Copy link

Custom attributes seems like a good workaround for selecting elements but what about when you are testing for the class name itself? Is there a way to test that the class name just contains a word?

For example:cy.get('[data-cy=div').should('have.class', 'active')
won't work because the class name would have a random hash at the end like active__3h43

@kuceb
Copy link
Contributor

kuceb commented Jul 9, 2018

@andrewdang17 this should work:
cy.get('...').invoke('attr', 'class').should('contain', 'className')

@MariiaB
Copy link

MariiaB commented Jul 29, 2018

@bkucera Your suggestion works perfectly in assertions, thanks.
Any ideas how to deal with dynamic names in "if statements" in conditional testing?
For example:

cy.get('[data-e2e="companiesFilter"]').then(($curEl) => {
    if($curEl.hasClass('togglerCollapsed')) {
      cy.get('[data-e2e="companiesFilterHeader"]').click();
    }
});

doesn't work because class name is class="togglerCollapsed___1kw8a toggler___3zlB6"

@kuceb
Copy link
Contributor

kuceb commented Jul 30, 2018

@MariiaB sure, you can get the class names with .attr('class') and then use includes('..')

if (cy.$$('[data-e2e="companiesFilter"]').attr('class').includes('togglerCollapsed')) {
      // your code here 
    }

@danielkcz
Copy link

danielkcz commented Aug 9, 2018

I am curious. Since custom data- attributes is a recommended way, why not to include some utility functions/commands for that? I mean it's easy to define a custom command that does it, but everyone has to find out first how to do that instead of just grab a function and use it. In my opinion, having this more build in would ease initial traction on the recommended approach. I would almost say it should be a first-class citizen having such function so people don't get burned by doing cy.get('button')

@pawellesniowski
Copy link

pawellesniowski commented Sep 18, 2018

@bkucera Thank you! I works. ;-)

const checkClassPresence = (element, className) => {
  cy
    .get(element)
    .invoke('attr', 'class')
    .should('contain', className);
};

@arshmakker
Copy link

arshmakker commented Jan 7, 2019

Hi @bkucera

I tried your approach

cy.get('div').invoke('attr', 'class').should('match', /MuiSelect-select-/);

but i think it would be a lot of performance hit and also it is not working :(

Also the second approach of putting custom data attributes, doesn't sound too good as it would make me add attributes just to test things.

In Jest, it is easier, as we can always import the component in the test framework and compare the snapshot with the rendered output. example:

// Link.react.test.js
import React from 'react';
import Link from '../Link.react';
import renderer from 'react-test-renderer';

test('Link changes the class when hovered', () => {
const component = renderer.create(
Facebook,
);
let tree = component.toJSON();
expect(tree).toMatchSnapshot();

// manually trigger the callback
tree.props.onMouseEnter();
// re-rendering
tree = component.toJSON();
expect(tree).toMatchSnapshot();

// manually trigger the callback
tree.props.onMouseLeave();
// re-rendering
tree = component.toJSON();
expect(tree).toMatchSnapshot();
});

courtesy: https://jestjs.io/docs/en/tutorial-react

but yes Jest is specific to the frameworks like React/Vue and Angular. I hate the fact that it lacks the nice integration platform which is offered by cypress. +1 to cypress for that.
PS: I am using material-ui for rendering React controls.

@bahmutov
Copy link
Contributor

bahmutov commented Jan 7, 2019

@danielkcz
Copy link

danielkcz commented Jan 7, 2019

The thing about data-testid is problematic for a custom component. Imagine you have a composition of like 2-3 components where only the last one in the chain is actually rendering any DOM. Adding hardcoded data-testid to this down level component is not always feasible as there might be more of those on the page. So it ends up with passing that ID through-out other components and it's rather messy.

Partial matching of classnames will surely have a performance hit, so I rather not...

This needs some new thinking out of the box. For example, some code post-processing plugins that would take care of this shenanigans in the background. I am not sure. It kinda makes me avoid testing these problematic scenarios which are a shame. Seeing this on Cypress landing page, it makes me 😢

image

@bahmutov
Copy link
Contributor

bahmutov commented Jan 7, 2019

@FredyC feel free to use data attribute or partial class name match like we show, it is up to you. Whatever you think will be maintainable in the long term and less likely to break. As far as performance - you are literally testing your code by loading the app again and gain in each test. Slightly less optimal selector would NOT even be close to loading the application over the network or affecting the speed of the test.

We suggest best practices for selectors in https://on.cypress.io/best-practices#Selecting-Elements and I personally recommend using https://github.com/kentcdodds/cypress-testing-library too

@danielkcz
Copy link

@bahmutov I've explained the problem of the data attribute, not sure what you don't understand about that. Also, I am using Emotion and there are not even remotely accessible class names to match against.

@bahmutov
Copy link
Contributor

bahmutov commented Jan 7, 2019

Sure @FredyC I understand - you are using 3rd party library that does not allow adding data attributes and does not have good class names. Well, what can Cypress do besides giving you cy.contains? If you don't expose good selectors, there are no good selectors. I think what would make you happy is a React-specific plugin for Cypress that would allow you to select by React component name, something like cy.react.get('<MyComponent>') or something similar

@kuceb
Copy link
Contributor

kuceb commented Jan 7, 2019

@FredyC might not help you in prod, but you can try this package https://github.com/lttb/reselector to automatically add data attrs from component name

edit: I believe it assigns an internal id to each component

@danielkcz
Copy link

danielkcz commented Jan 7, 2019

Sure @FredyC I understand - you are using 3rd party library that does not allow adding data attributes and does not have good class names.

Not only 3rd party components. Imagine a situation there is a generic Button component which actually renders some markup. Then there is PrimaryButton which adds some styling to that Button. Lastly, there can be EditButton which adds some extra logic to above PrimaryButton. To get data attribute down to the Button, I would have to pass it through all those components. And this is a contrived example. In bigger apps, it can get rather nasty.

Well, what can Cypress do besides giving you cy.contains?

I am not saying it's a fault of Cypress (or anyones), I am mostly trying to open discussion if there isn't some better way. Writing tests shouldn't be hard, right?

If you don't expose good selectors, there are no good selectors. I think what would make you happy is a React-specific plugin for Cypress that would allow you to select by React component name, something like cy.react.get('<MyComponent>') or something similar

Not sure how that could work considering you have only the DOM at your disposal. If it's somehow doable then it would definitely improve things a lot in my opinion. Although it would probably need to support selectors as well because only the name of the component doesn't need to be unique. Or would it be chainable with regular selectors? It's really intriguing idea :)

@JonJamesDesign
Copy link

JonJamesDesign commented Jun 23, 2019

If classes need to be dynamic, they should be set based on state.

E.g. using CSS Modules:

import React, { useState } from 'react'
import css from './Menu.module.css'

const Menu = () => {
  const [menuOpen, setMenuOpen] = useState(false)

  const _toggleMenu = () => {
    const newMenuOpenState = !menuOpen
    setMenuOpen(newMenuOpenState)
  }

  return (
    <>
      <button onClick={_toggleMenu}>Open / Close Menu</button>
      <nav className={`${css.menu} ${menuOpen ? css.menuOpen : css.menuClosed}`} />
    </>
  )
}

@ModestasDaunys
Copy link

@andrewdang17 this should work:
cy.get('...').invoke('attr', 'class').should('contain', 'className')

This saved my life. Thank you.

@maemreyo
Copy link

I had stuck with this when working with Cypress, but I found this on the document of Cypress: https://docs.cypress.io/api/commands/get#Find-the-link-with-an-href-attribute-containing-the-word-questions-and-click-it
We can use this for the class attribute instead of adding additional testid (or other id on the element)
For example:
cy.get('div[class*="container"]').should('be.visible');

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

No branches or pull requests