Skip to content

Commit

Permalink
Fixed a regression that could cause a crash when looking for an ignor…
Browse files Browse the repository at this point in the history 
…ing comment for unsafe pseudo-classes (#2864)

* Fixed a regression that could cause a crash when looking for an ignoring comment for unsafe pseudo-classes

* Fix flow types
  • Loading branch information
@Andarist
Andarist committed Aug 23, 2022
1 parent c60d646 commit b9b8b74
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/polite-points-study.md
View file
@@ -0,0 +1,5 @@
---
'@emotion/cache': patch
---

Fixed a regression that could cause a crash when looking for an ignoring comment for unsafe pseudo-classes.
12 changes: 5 additions & 7 deletions packages/cache/src/stylis-plugins.js
View file
Expand Up @@ -186,15 +186,15 @@ export let createUnsafeSelectorsAlarm = cache => (element, index, children) => {
: // global rule at the root level
children

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

if (node.line > element.line) {
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
// so we seek *from the end* for the node that is earlier than the rule's `element` and check that
// this will also match inputs like this:
// .a {
// /* comm */
Expand All @@ -209,10 +209,8 @@ export let createUnsafeSelectorsAlarm = cache => (element, index, children) => {
// }
// 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)) {
if (node.column < element.column) {
if (isIgnoringComment(node)) {
return
}
break
Expand Down
58 changes: 58 additions & 0 deletions packages/react/__tests__/__snapshots__/warnings.js.snap
View file
Expand Up @@ -90,6 +90,64 @@ 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 */ does warn when not 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 warn when not 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 */ does warn when not 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 */ does warn when not 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 */ does warn when not 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 Down
125 changes: 124 additions & 1 deletion packages/react/__tests__/warnings.js
View file
Expand Up @@ -126,6 +126,129 @@ describe('unsafe pseudo classes', () => {
})
})

test('does warn when not using the flag on the rule that follows another rule', () => {
expect(
renderer
.create(
<div
css={{
'& > *': {
marginLeft: 10
},
[`& > *:first-child$`]: {
marginLeft: 0
}
}}
/>
)
.toJSON()
).toMatchSnapshot()
expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
[
[
"The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".",
],
]
`)
})

test('does warn when not using the flag on the rule that preceeds another rule', () => {
expect(
renderer
.create(
<div
css={{
[`& > *:first-child`]: {
marginLeft: 0
},
'& > *': {
marginLeft: 10
}
}}
/>
)
.toJSON()
).toMatchSnapshot()
expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
[
[
"The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".",
],
]
`)
})

test('does warn when not using the flag on the rule that follows a declaration', () => {
expect(
renderer
.create(
<div
css={{
color: 'hotpink',
[`& > *:first-child`]: {
marginLeft: 0
}
}}
/>
)
.toJSON()
).toMatchSnapshot()
expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
[
[
"The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".",
],
]
`)
})

test('does warn when not using the flag on the rule that preceeds a declaration', () => {
expect(
renderer
.create(
<div
css={{
[`& > *:first-child`]: {
marginLeft: 0
},
color: 'hotpink'
}}
/>
)
.toJSON()
).toMatchSnapshot()
expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
[
[
"The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".",
],
]
`)
})

test('does warn when not using the flag on a global rule', () => {
expect(
renderer
.create(
<Global
styles={{
[`body > *:first-child`]: {
marginLeft: 0
}
}}
/>
)
.toJSON()
).toMatchSnapshot()
expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
[
[
"The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".",
],
]
`)
})

test('does not warn when using the flag on the rule that follows another rule', () => {
expect(
renderer
Expand Down Expand Up @@ -184,7 +307,7 @@ describe('unsafe pseudo classes', () => {
expect(console.error).not.toBeCalled()
})

test.only('does not warn when using the flag on the rule that preceeds a declaration', () => {
test('does not warn when using the flag on the rule that preceeds a declaration', () => {
expect(
renderer
.create(
Expand Down

0 comments on commit b9b8b74

Please sign in to comment.