Merge pull request tailwindlabs#209 from tailwindcss/error-on-media-apply

[0.2] Be more strict about which classes can be `@apply`'d

Author: adamwathan

__tests__/applyAtRule.test.js

@@ -14,39 +14,64 @@ test("it copies a class's declarations into itself", () => {
   })
 })
 
-test("it doesn't copy a media query definition into itself", () => {
-  const output = `.a {
-            color: red;
-        }
-
-        @media (min-width: 300px) {
-            .a { color: blue; }
-        }
-
-        .b {
-            color: red;
-        }`
-
-  return run(
-    `.a {
-            color: red;
-        }
-
-        @media (min-width: 300px) {
-            .a { color: blue; }
-        }
-
-        .b {
-            @apply .a;
-        }`
-  ).then(result => {
-    expect(result.css).toEqual(output)
-    expect(result.warnings().length).toBe(0)
+test('it fails if the class does not exist', () => {
+  return run('.b { @apply .a; }').catch(e => {
+    expect(e).toMatchObject({ name: 'CssSyntaxError' })
   })
 })
 
-test('it fails if the class does not exist', () => {
-  run('.b { @apply .a; }').catch(error => {
-    expect(error.reason).toEqual('No .a class found.')
+test('applying classes that are ever used in a media query is not supported', () => {
+  const input = `
+    .a {
+      color: red;
+    }
+
+    @media (min-width: 300px) {
+      .a { color: blue; }
+    }
+
+    .b {
+      @apply .a;
+    }
+  `
+  expect.assertions(1)
+  return run(input).catch(e => {
+    expect(e).toMatchObject({ name: 'CssSyntaxError' })
+  })
+})
+
+test('it does not match classes that include pseudo-selectors', () => {
+  const input = `
+    .a:hover {
+      color: red;
+    }
+
+    .b {
+      @apply .a;
+    }
+  `
+  expect.assertions(1)
+  return run(input).catch(e => {
+    expect(e).toMatchObject({ name: 'CssSyntaxError' })
+  })
+})
+
+test('it does not match classes that have multiple rules', () => {
+  const input = `
+    .a {
+      color: red;
+    }
+
+    .b {
+      @apply .a;
+    }
+
+    .a {
+      color: blue;
+    }
+  `
+  expect.assertions(1)
+  return run(input).catch(e => {
+    expect(e).toMatchObject({ name: 'CssSyntaxError' })
   })
 })

src/lib/substituteClassApplyAtRules.js

@@ -1,6 +1,5 @@
 import _ from 'lodash'
 import postcss from 'postcss'
-import findMixin from '../util/findMixin'
 import escapeClassName from '../util/escapeClassName'
 
 function normalizeClassNames(classNames) {
@@ -9,6 +8,33 @@ function normalizeClassNames(classNames) {
   })
 }
 
+function findMixin(css, mixin, onError) {
+  const matches = []
+
+  css.walkRules(rule => {
+    if (rule.selectors.includes(mixin)) {
+      if (rule.parent.type !== 'root') {
+        // prettier-ignore
+        onError(`\`@apply\` cannot be used with ${mixin} because ${mixin} is nested inside of an at-rule (@${rule.parent.name}).`)
+      }
+
+      matches.push(rule)
+    }
+  })
+
+  if (_.isEmpty(matches)) {
+    // prettier-ignore
+    onError(`\`@apply\` cannot be used with ${mixin} because ${mixin} either does not exist, or it's actual definition includes a pseudo-class like :hover, :active, etc.`)
+  }
+
+  if (matches.length > 1) {
+    // prettier-ignore
+    onError(`\`@apply\` cannot be used with ${mixin} because ${mixin} is included in multiple rulesets.`)
+  }
+
+  return _.flatten(matches.map(match => match.clone().nodes))
+}
+
 export default function() {
   return function(css) {
     css.walkRules(rule => {
@@ -27,8 +53,8 @@ export default function() {
         })
 
         const decls = _.flatMap(classes, mixin => {
-          return findMixin(css, mixin, () => {
-            throw atRule.error(`No ${mixin} class found.`)
+          return findMixin(css, mixin, message => {
+            throw atRule.error(message)
           })
         })