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.

Sign up for GitHub

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

Fix ignoreFlag not works bug #2751

Merged
merged 8 commits into from
Aug 22, 2022
Merged

Fix ignoreFlag not works bug #2751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ebf668
fix ignoreFlag not works
May 17, 2022
489488e
Add changeset
May 17, 2022
4093894
Fix for loop syntax
May 17, 2022
7a10769
Update .changeset/witty-frogs-hear.md
kyoncy May 18, 2022
7ba9bbb
Merge branch 'main' into main
kyoncy Aug 18, 2022
9a53e9c
Refactor looping test to .each
Andarist Aug 17, 2022
e2fdced
Fixed the issue with ignore comment detection
Andarist Aug 17, 2022
b82bcee
add changeset attributions
Andarist Aug 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/witty-frogs-hear.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@emotion/cache': patch
---

author: @Andarist
author: @kyoncy

Fix `/* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */` not disabling the warning when there are multiple blocks of CSS.
64 changes: 56 additions & 8 deletions packages/cache/src/stylis-plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,70 @@ const ignoreFlag =
'emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason'

const isIgnoringComment = element =>
!!element &&
element.type === 'comm' &&
element.children.indexOf(ignoreFlag) > -1
element.type === 'comm' && element.children.indexOf(ignoreFlag) > -1

export let createUnsafeSelectorsAlarm = cache => (element, index, children) => {
if (element.type !== 'rule') return
if (element.type !== 'rule' || cache.compat) return

const unsafePseudoClasses = element.value.match(
/(:first|:nth|:nth-last)-child/g
)

if (unsafePseudoClasses && cache.compat !== true) {
const prevElement = index > 0 ? children[index - 1] : null
if (prevElement && isIgnoringComment(last(prevElement.children))) {
return
if (unsafePseudoClasses) {
const isNested = element.parent === children[0]
// in nested rules comments become children of the "auto-inserted" rule
//
// considering this input:
// .a {
// .b /* comm */ {}
// color: hotpink;
// }
// we get output corresponding to this:
// .a {
// & {
// /* comm */
// color: hotpink;
// }
// .b {}
// }
const commentContainer = isNested
? children[0].children
: // global rule at the root level
children

for (let i = 0; i < commentContainer.length; i++) {
const node = commentContainer[i]

if (node.line > element.line) {
break
}

// it is quite weird but comments are *usually* put at `column: element.column - 1`
// so we seek for the node that is later than the rule's `element` and check the previous element
// this will also match inputs like this:
// .a {
// /* comm */
// .b {}
// }
//
// but that is fine
//
// it would be the easiest to change the placement of the comment to be the first child of the rule:
// .a {
// .b { /* comm */ }
// }
// with such inputs we wouldn't have to search for the comment at all
// TODO: consider changing this comment placement in the next major version
if (node.column > element.column) {
const previousNode = commentContainer[i - 1]

if (isIgnoringComment(previousNode)) {
return
}
break
}
}

unsafePseudoClasses.forEach(unsafePseudoClass => {
console.error(
`The pseudo class "${unsafePseudoClass}" is potentially unsafe when doing server-side rendering. Try changing it to "${
Expand Down
138 changes: 98 additions & 40 deletions packages/react/__tests__/__snapshots__/warnings.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,37 +32,65 @@ exports[`does not warn when valid values are passed for the content property 1`]

exports[`global with css prop 1`] = `null`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child /* [flag] */" in a style object 1`] = `
.emotion-0:first-child {
color: rebeccapurple;
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ does not warn when using the flag on a global rule 1`] = `null`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ does not warn when using the flag on the rule that follows a declaration 1`] = `
.emotion-0 {
color: hotpink;
}

.emotion-0>*:first-child {
margin-left: 0;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child /* [flag] */" in a style string 1`] = `
.emotion-0:first-child {
color: rebeccapurple;
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ does not warn when using the flag on the rule that follows another rule 1`] = `
.emotion-0>* {
margin-left: 10px;
}

.emotion-0>*:first-child {
margin-left: 0;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child :nth-child(3) /* [flag] */" in a style object 1`] = `
.emotion-0:first-child :nth-child(3) {
color: rebeccapurple;
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ does not warn when using the flag on the rule that preceeds a declaration 1`] = `
.emotion-0 {
color: hotpink;
}

.emotion-0>*:first-child {
margin-left: 0;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child :nth-child(3) /* [flag] */" in a style string 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ does not warn when using the flag on the rule that preceeds another rule 1`] = `
.emotion-0>*:first-child {
margin-left: 0;
}

.emotion-0>* {
margin-left: 10px;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child :nth-child(3) 1`] = `
.emotion-0:first-child :nth-child(3) {
color: rebeccapurple;
}
Expand All @@ -72,8 +100,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child span /* [flag] */" in a style object 1`] = `
.emotion-0:first-child span {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child 1`] = `
.emotion-0:first-child {
color: rebeccapurple;
}

Expand All @@ -82,7 +110,7 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child span /* [flag] */" in a style string 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child span 1`] = `
.emotion-0:first-child span {
color: rebeccapurple;
}
Expand All @@ -92,7 +120,7 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child, :nth-child(3) /* [flag] */" in a style object 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child, :nth-child(3) 1`] = `
.emotion-0:first-child,
.emotion-0:nth-child(3) {
color: rebeccapurple;
Expand All @@ -103,9 +131,9 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child, :nth-child(3) /* [flag] */" in a style string 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child, span 1`] = `
.emotion-0:first-child,
.emotion-0:nth-child(3) {
.emotion-0 span {
color: rebeccapurple;
}

Expand All @@ -114,9 +142,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child, span /* [flag] */" in a style object 1`] = `
.emotion-0:first-child,
.emotion-0 span {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :first-child:nth-child(3) 1`] = `
.emotion-0:first-child:nth-child(3) {
color: rebeccapurple;
}

Expand All @@ -125,9 +152,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child, span /* [flag] */" in a style string 1`] = `
.emotion-0:first-child,
.emotion-0 span {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :not(:first-child) 1`] = `
.emotion-0:not(:first-child) {
color: rebeccapurple;
}

Expand All @@ -136,8 +162,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child:nth-child(3) /* [flag] */" in a style object 1`] = `
.emotion-0:first-child:nth-child(3) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :not(:nth-child(3)) 1`] = `
.emotion-0:not(:nth-child(3)) {
color: rebeccapurple;
}

Expand All @@ -146,8 +172,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":first-child:nth-child(3) /* [flag] */" in a style string 1`] = `
.emotion-0:first-child:nth-child(3) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :nth-child(3) 1`] = `
.emotion-0:nth-child(3) {
color: rebeccapurple;
}

Expand All @@ -156,8 +182,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":not(:first-child) /* [flag] */" in a style object 1`] = `
.emotion-0:not(:first-child) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with object styles :nth-last-child(7) 1`] = `
.emotion-0:nth-last-child(7) {
color: rebeccapurple;
}

Expand All @@ -166,8 +192,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":not(:first-child) /* [flag] */" in a style string 1`] = `
.emotion-0:not(:first-child) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child :nth-child(3) 1`] = `
.emotion-0:first-child :nth-child(3) {
color: rebeccapurple;
}

Expand All @@ -176,8 +202,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":not(:nth-child(3)) /* [flag] */" in a style object 1`] = `
.emotion-0:not(:nth-child(3)) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child 1`] = `
.emotion-0:first-child {
color: rebeccapurple;
}

Expand All @@ -186,8 +212,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":not(:nth-child(3)) /* [flag] */" in a style string 1`] = `
.emotion-0:not(:nth-child(3)) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child span 1`] = `
.emotion-0:first-child span {
color: rebeccapurple;
}

Expand All @@ -196,7 +222,8 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":nth-child(3) /* [flag] */" in a style object 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child, :nth-child(3) 1`] = `
.emotion-0:first-child,
.emotion-0:nth-child(3) {
color: rebeccapurple;
}
Expand All @@ -206,8 +233,9 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":nth-child(3) /* [flag] */" in a style string 1`] = `
.emotion-0:nth-child(3) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child, span 1`] = `
.emotion-0:first-child,
.emotion-0 span {
color: rebeccapurple;
}

Expand All @@ -216,8 +244,38 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":nth-last-child(7) /* [flag] */" in a style object 1`] = `
.emotion-0:nth-last-child(7) {
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :first-child:nth-child(3) 1`] = `
.emotion-0:first-child:nth-child(3) {
color: rebeccapurple;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :not(:first-child) 1`] = `
.emotion-0:not(:first-child) {
color: rebeccapurple;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :not(:nth-child(3)) 1`] = `
.emotion-0:not(:nth-child(3)) {
color: rebeccapurple;
}

<div
className="emotion-0"
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :nth-child(3) 1`] = `
.emotion-0:nth-child(3) {
color: rebeccapurple;
}

Expand All @@ -226,7 +284,7 @@ exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-di
/>
`;

exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ ":nth-last-child(7) /* [flag] */" in a style string 1`] = `
exports[`unsafe pseudo classes does not warn when using with flag: /* emotion-disable-server-rendering-unsafe-selector-warning-please-do-not-use-this-the-warning-exists-for-a-reason */ with string styles :nth-last-child(7) 1`] = `
.emotion-0:nth-last-child(7) {
color: rebeccapurple;
}
Expand Down
Loading