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

core: attribute selectors with comma not working #377

Open
igorbt opened this issue May 19, 2023 · 3 comments
Open

core: attribute selectors with comma not working #377

igorbt opened this issue May 19, 2023 · 3 comments
Labels
🐞 bug Something isn't working

Comments

@igorbt
Copy link

igorbt commented May 19, 2023

When using nested attribute selectors that contains commas, they generate incorrect styles, inserting ampersands after the comma.

For example:

    '& [style*="color: rgb(245, 240, 241);"]': {
      color: 'black !important'
    },

will result in something like

.fk7b603 [style*="color: rgb(245,& 240,& 241);"] { color: black !important; }

See more in this Codesandbox.

@igorbt igorbt changed the title Attribute selectors with comma now working Attribute selectors with comma not working May 19, 2023
@layershifter
Copy link
Member

layershifter commented May 19, 2023

That indeed a bug, can be reproed in our sandbox, too.

export function normalizePseudoSelector(pseudoSelector: string): string {
return (
'&' +
normalizeNestedProperty(
// Regex there replaces a comma, spaces and an ampersand if it's present with comma and an ampersand.
// This allows to normalize input, see examples in JSDoc.
pseudoSelector.replace(PSEUDO_SELECTOR_REGEX, ',&$1'),
)
);
}

The function above is probably the source of the problem.

(I hope that such CSS rules are not used in real products are they will cause performance issues).

@layershifter layershifter changed the title Attribute selectors with comma not working bug(core): attribute selectors with comma not working May 19, 2023
@layershifter layershifter added the 🐞 bug Something isn't working label May 19, 2023
@igorbt
Copy link
Author

igorbt commented May 19, 2023

Thank you @layershifter ! I would definitely not use such rules in normal conditions. But sometimes, for complex and legacy apps that could be a good workaround.
BTW, can you elaborate about the performance issues? I understand that this selector is less performant than selecting by class, but do you think the performance hit will be noticeable?

@layershifter
Copy link
Member

layershifter commented May 19, 2023

BTW, can you elaborate about the performance issues? I understand that this selector is less performant than selecting by class, but do you think the performance hit will be noticeable?

It will be noticeable if an app has a big DOM tree. In general:

.foo [style*="color: red"] { color: pink }
image

(Use Edge with enabled paint instrumentation)
https://codesandbox.io/s/blissful-mountain-ir406p?file=/src/App.js

Note1: That selector will affect only elements that have inline styles, so it might not be so bad, but it still impacts performance
Note2: I don't know what optimisations browsers will apply in a particular case, but it's how it looks in general.

@layershifter layershifter changed the title bug(core): attribute selectors with comma not working core: attribute selectors with comma not working Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants