Skip to content

Commit

Permalink
Improve error message about incorrect @import insertion
Browse files Browse the repository at this point in the history
  • Loading branch information
Andarist committed Jun 14, 2020
1 parent 5e39d81 commit a1d7173
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 7 deletions.
13 changes: 13 additions & 0 deletions packages/cache/src/stylis-plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,17 @@ const isPrependedWithRegularRules = (index, children) => {
return false
}

// use this to remove incorrect elements from further processing
// so they don't get handed to the `sheet` (or anything else)
// as that could potentially lead to additional logs which in turn could be overhelming to the user
const nullifyElement = element => {
element.type = ''
element.value = ''
element.return = ''
element.children = ''
element.props = ''
}

export let incorrectImportAlarm = (element, index, children) => {
if (!isImportRule(element)) {
return
Expand All @@ -149,9 +160,11 @@ export let incorrectImportAlarm = (element, index, children) => {
console.error(
"`@import` rules can't be nested inside other rules. Please move it to the top and put it before regular rules. Keep in mind that they can only be used within global styles."
)
nullifyElement(element)
} else if (isPrependedWithRegularRules(index, children)) {
console.error(
"`@import` rules can't be preceded by regular rules. Please put it before them."
)
nullifyElement(element)
}
}
19 changes: 19 additions & 0 deletions packages/css/test/warnings.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @flow
import 'test-utils/legacy-env'
import { css } from '@emotion/css'
import createCss from '@emotion/css/create-instance'
import * as React from 'react'
import renderer from 'react-test-renderer'

Expand Down Expand Up @@ -53,3 +54,21 @@ it('does warn when functions are passed to css calls', () => {
"Functions that are interpolated in css calls will be stringified.\nIf you want to have a css call based on props, create a function that returns a css call like this\nlet dynamicStyle = (props) => css`color: ${props.color}`\nIt can be called directly with props or interpolated in a styled call like this\nlet SomeComponent = styled('div')`${dynamicStyle}`"
)
})

it('does warn when @import rule is being inserted after order-insensitive rules', () => {
const { injectGlobal } = createCss({ key: 'import-after-regular' })

injectGlobal`.thing {display:flex;}`
injectGlobal`@import 'custom.css';`

expect((console.error: any).mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"You have attempted inserting such rule:
@import 'custom.css';
\`@import\` rules must precede all other types of rules in a stylesheet. Please ensure that you insert them first.",
],
]
`)
})
32 changes: 25 additions & 7 deletions packages/sheet/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ export class StyleSheet {
}
const tag = this.tags[this.tags.length - 1]

if (process.env.NODE_ENV !== 'production') {
const isImportRule =
rule.charCodeAt(0) === 64 && rule.charCodeAt(1) === 105

// $FlowFixMe
if (isImportRule && this._alreadyInsertedOrderInsensitiveRule) {
// this would only cause problem in speedy mode
// but we don't want enabling speedy to affect the observable behavior
// so we report this error at all times
console.error(
`You have attempted inserting such rule:\n` +
rule +
'\n\n`@import` rules must precede all other types of rules in a stylesheet. Please ensure that you insert them first.'
)
}

// $FlowFixMe
this._alreadyInsertedOrderInsensitiveRule =
this._alreadyInsertedOrderInsensitiveRule || !isImportRule
}

if (this.isSpeedy) {
const sheet = sheetForTag(tag)
try {
Expand All @@ -120,13 +141,6 @@ export class StyleSheet {
`There was a problem inserting the following rule: "${rule}"`,
e
)
let isImportRule =
rule.charCodeAt(0) === 64 && rule.charCodeAt(1) === 105
if (isImportRule) {
console.error(
'`@import` rules must be inserted before other rules.'
)
}
}
}
} else {
Expand All @@ -140,5 +154,9 @@ export class StyleSheet {
this.tags.forEach(tag => tag.parentNode.removeChild(tag))
this.tags = []
this.ctr = 0
if (process.env.NODE_ENV !== 'production') {
// $FlowFixMe
this._alreadyInsertedOrderInsensitiveRule = false
}
}
}

0 comments on commit a1d7173

Please sign in to comment.