Enzyme: Support escaped characters in selectors

Created on 3 Oct 2017  ยท  14Comments  ยท  Source: enzymejs/enzyme

Edit from maintainers: this issue was repurposed, see https://github.com/airbnb/enzyme/issues/1218#issuecomment-334489966

Settings

  • React v16
  • Enzyme v3

When i have a component which has a className prop with special character / or @ , it doesnt select anything. I feel enzyme breaks and doesn't continue with the lines after this selector.

Example

const sampleComponent = () => (
    <div className="u-width-1/2@small">
        Something here
    </div>
);

If I have the above component and if I want to select the div containing u-width-1/2@small I tried

const component = shallow(<sampleComponent />);
component.find('.u-width-1/2@small') // This line seems to fail due to error in selector
/* Anything after this doesnt execute */
feature request help wanted

Most helpful comment

I agree with @yash0087 and @coderas, a class with an @ symbol (or anything other than whitespace for that matter) is indeed a valid class name.

It's worth noting the class names are defined by the HTML spec rather than CSS โ€“ though the CSS spec defines what is a valid class selector as per @yash0087's link above. The HTML5 spec clearly states that classes are simply "a set of space-separated tokens", which have no restrictions other than whitespace characters: https://www.w3.org/TR/2011/WD-html5-20110525/elements.html#classes

In addition, the argument that document.querySelector does not allow those characters is also false. document.querySelector uses the Selectors API, which explicitly states that the same escaping rules as CSS should be applied for special characters: https://drafts.csswg.org/selectors-4/#case-sensitive.

All 14 comments

Following is the error thrown

Failed to parse selector: .u-width-1/2@small

at safelyGenerateTokens (node_modules/enzyme/build/selectors.js:75:11)
      at reduceTreeBySelector (node_modules/enzyme/build/selectors.js:286:18)
      at node_modules/enzyme/build/selectors.js:352:12
      at Array.map (native)
      at reduceTreesBySelector (node_modules/enzyme/build/selectors.js:351:23)
      at ShallowWrapper.find (node_modules/enzyme/build/ShallowWrapper.js:719:63)
      at Object.<anonymous> (src/components/ProductsLayout.spec.js:68:48)
      at Promise.resolve.then.el (node_modules/p-map/index.js:46:16)
      at process._tickCallback (internal/process/next_tick.js:109:7)

That's not a valid class name - that won't work in the browser either.

Yup, and I don't think we'll be supporting non-standard characters in selectors. If you're ever wondering if a selector is valid, try passing it to document.querySelector. If it throws an error saying it's not a valid selector, then it's not a selector we will support.

There are a few exceptions to that, such as allowing literal numeric values, but as far as selector names go they should be in spec.

@aweary and @ljharb This is a bit of a surprise as none of my CSS class names have changed for more than a year and it works perfectly in the browser. In Enzyme v2 it worked as expected.

With regards to valid CSS names, as per this doc
(https://www.w3.org/TR/CSS21/syndata.html#characters), valid names can be [a-zA-Z0-9] and ISO 10646 characters U+00A0 and higher, plus the hyphen (-) and the underscore (_). I don't see why we consider / and @ to be invalid?

@ljharb that's simply not true, this is an issue. classes such as u-width-1/4@medium are supported in modern browsers, and are valid.

@aweary - Your test is also not valid, since the query does throw and error (if unescaped), but the styles are applied:

Straight from chrome inspector, my local project:

@media (min-width: 61.25em)
<style>โ€ฆ</style>
.u-width-1\/2\@large {
    width: 50%;
}

This coupled with what @yash0087 has highlighted from the spec is enough argument I feel to reopen the issue.

Have a play around, browsers style to these classes, that are often used in frameworks these days.

(escaped in CSS, not in the DOM element attribute)

I agree with @yash0087 and @coderas, a class with an @ symbol (or anything other than whitespace for that matter) is indeed a valid class name.

It's worth noting the class names are defined by the HTML spec rather than CSS โ€“ though the CSS spec defines what is a valid class selector as per @yash0087's link above. The HTML5 spec clearly states that classes are simply "a set of space-separated tokens", which have no restrictions other than whitespace characters: https://www.w3.org/TR/2011/WD-html5-20110525/elements.html#classes

In addition, the argument that document.querySelector does not allow those characters is also false. document.querySelector uses the Selectors API, which explicitly states that the same escaping rules as CSS should be applied for special characters: https://drafts.csswg.org/selectors-4/#case-sensitive.

It's worth noting the class names are defined by the HTML spec rather than CSS โ€“ though the CSS spec defines what is a valid class selector as per @yash0087's link above.

Your test is also not valid, since the query does throw and error (if unescaped), but the styles are applied:

@coderas @aaronthomas The definition of a valid class name and a valid selector are independent (for whatever reason). Our selector parser aims to implement the grammar for the Selector API specifically.

In addition, the argument that document.querySelector does not allow those characters is also false.

It's absolutely not false. Feel free to try it yourself. A selector like .foo@small will fail as an invalid selector.

Uncaught DOMException: Failed to execute 'querySelector' on 'Document': '.foo@small' is not a valid selector.
    at <anonymous>:1:10
(anonymous) @ VM637:1

which explicitly states that the same escaping rules as CSS should be applied for special characters: https://drafts.csswg.org/selectors-4/#case-sensitive.

I agree we should support these characters when escaped, but that's explicitly _not_ what @SathishGovindaraju opened this issue about. It was reported with the selector component.find('.u-width-1/2@small') which does not contain any sort of escape sequence. The grammar for a selector consists of alphabetic and non-ASCII characters as defined here: https://drafts.csswg.org/css-syntax-3/#ident-token-diagram

Feel free to submit a PR to rst-selector-parser implementing arbitrary escaped characters, but we won't support using those characters without escape sequences. When you ask to support:

a class with an @ symbol (or anything other than whitespace for that matter)

It ends up defining an ambiguous grammar in some cases (Is .foo#bar a class named foo#bar or a class .foo and ID bar?) which isn't acceptable. This is explicitly why the spec does _not_ allow those characters without escaping.

As a workaround you can use findWhere for now.

@coderas specifically '.foo\\@small' will work, since the evaluated string has to contain the escape sequence. Parsing arbitrary escape sequences is a hard problem ๐Ÿ˜‰ As I mentioned, feel free to submit a PR to rst-selector-parser fixing this, I'd be happy to review.

edit: comment was deleted ๐Ÿ‘€

Renaming and re-opening ๐Ÿ‘

I'm also having this problem with classes that include the + sign, for example .button .+primary

I'm also having this problem with selectors like .+foo

@joeldenning i don't think that's a valid selectable class name; document.querySelectorAll('.+foo') throws for me.

@ljharb I'd start to work on this. If I didn't get the comments wrong we have to make escape string work in rst-selector-parser right?

@chenesan yes, ideally anything supported by querySelector should work.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aweary picture aweary  ยท  3Comments

dschinkel picture dschinkel  ยท  3Comments

ahuth picture ahuth  ยท  3Comments

AdamYahid picture AdamYahid  ยท  3Comments

nelsonchen90 picture nelsonchen90  ยท  3Comments