Allow variant plugins to tell Tailwind they should stack (#2382)

adamwathan · JNavith · web-flow · commit 32eac198582f · 2020-09-14T09:12:36.000-04:00
* Fix unwanted stacking behavior on any non-darkModeVariant "dark" variant (#2380) * Add failing tests for non-darkModeVariant "dark" variant stacking behavior * Fix unwanted non-darkModeVariant "dark" variant stacking (by making the failing test pass) * Add unstable_stack option for variants to tell Tailwind they should stack * Update eslint to allow unstable_ variables * Update changelog Co-authored-by: Navith <28162694+JakeNavith@users.noreply.github.com>
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -9,6 +9,7 @@
   "extends": ["eslint-config-postcss", "prettier"],
   "plugins": ["prettier"],
   "rules": {
+    "camelcase": ["error", { "allow": ["^unstable_"] }],
     "no-unused-vars": [2, { "args": "all", "argsIgnorePattern": "^_" }],
     "no-warning-comments": 0,
     "prettier/prettier": [
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-- Nothing yet!
+### Fixed
+
+- Prevent new `dark` experiment from causing third-party `dark` variants to inherit stacking behavior ([#2382](https://github.com/tailwindlabs/tailwindcss/pull/2382))
 
 ## [1.8.9] - 2020-09-13
 
diff --git a/__tests__/darkMode.test.js b/__tests__/darkMode.test.js
@@ -1,5 +1,6 @@
 import postcss from 'postcss'
 import tailwind from '../src/index'
+import createPlugin from '../src/util/createPlugin'
 
 function run(input, config = {}) {
   return postcss([tailwind({ experimental: { darkModeVariant: true }, ...config })]).process(
@@ -21,6 +22,45 @@ test('dark mode variants cannot be generated without enabling the dark mode expe
   return expect(run(input, { experimental: {} })).rejects.toThrow()
 })
 
+test('user-defined dark mode variants do not stack when the dark mode experiment is disabled', () => {
+  const input = `
+    @variants dark, hover {
+      .text-red {
+        color: red;
+      }
+    }
+  `
+
+  const expected = `
+    .text-red {
+      color: red;
+    }
+    .custom-dark .custom-dark\\:text-red {
+      color: red;
+    }
+    .hover\\:text-red:hover {
+      color: red;
+    }
+  `
+
+  const userPlugin = createPlugin(function({ addVariant }) {
+    addVariant('dark', function({ modifySelectors }) {
+      modifySelectors(function({ className }) {
+        return `.custom-dark .custom-dark\\:${className}`
+      })
+    })
+  })
+
+  expect.assertions(2)
+
+  return postcss([tailwind({ experimental: { darkModeVariant: false }, plugins: [userPlugin] })])
+    .process(input, { from: undefined })
+    .then(result => {
+      expect(result.css).toMatchCss(expected)
+      expect(result.warnings().length).toBe(0)
+    })
+})
+
 test('generating dark mode variants uses the media strategy by default', () => {
   const input = `
     @variants dark {
diff --git a/src/flagged/darkModeVariantPlugin.js b/src/flagged/darkModeVariantPlugin.js
@@ -1,38 +1,42 @@
 import buildSelectorVariant from '../util/buildSelectorVariant'
 
 export default function({ addVariant, config, postcss, prefix }) {
-  addVariant('dark', ({ container, separator, modifySelectors }) => {
-    if (config('dark') === 'media') {
-      const modified = modifySelectors(({ selector }) => {
-        return buildSelectorVariant(selector, 'dark', separator, message => {
-          throw container.error(message)
+  addVariant(
+    'dark',
+    ({ container, separator, modifySelectors }) => {
+      if (config('dark') === 'media') {
+        const modified = modifySelectors(({ selector }) => {
+          return buildSelectorVariant(selector, 'dark', separator, message => {
+            throw container.error(message)
+          })
         })
-      })
-      const mediaQuery = postcss.atRule({
-        name: 'media',
-        params: '(prefers-color-scheme: dark)',
-      })
-      mediaQuery.append(modified)
-      container.append(mediaQuery)
-      return container
-    }
+        const mediaQuery = postcss.atRule({
+          name: 'media',
+          params: '(prefers-color-scheme: dark)',
+        })
+        mediaQuery.append(modified)
+        container.append(mediaQuery)
+        return container
+      }
 
-    if (config('dark') === 'class') {
-      const modified = modifySelectors(({ selector }) => {
-        return buildSelectorVariant(selector, 'dark', separator, message => {
-          throw container.error(message)
+      if (config('dark') === 'class') {
+        const modified = modifySelectors(({ selector }) => {
+          return buildSelectorVariant(selector, 'dark', separator, message => {
+            throw container.error(message)
+          })
         })
-      })
 
-      modified.walkRules(rule => {
-        rule.selectors = rule.selectors.map(selector => {
-          return `${prefix('.dark')} ${selector}`
+        modified.walkRules(rule => {
+          rule.selectors = rule.selectors.map(selector => {
+            return `${prefix('.dark')} ${selector}`
+          })
         })
-      })
 
-      return modified
-    }
+        return modified
+      }
 
-    throw new Error("The `dark` config option must be either 'media' or 'class'.")
-  })
+      throw new Error("The `dark` config option must be either 'media' or 'class'.")
+    },
+    { unstable_stack: true }
+  )
 }
diff --git a/src/lib/substituteVariantsAtRules.js b/src/lib/substituteVariantsAtRules.js
@@ -24,32 +24,38 @@ function ensureIncludesDefault(variants) {
 
 const defaultVariantGenerators = config => ({
   default: generateVariantFunction(() => {}),
-  'motion-safe': generateVariantFunction(({ container, separator, modifySelectors }) => {
-    const modified = modifySelectors(({ selector }) => {
-      return buildSelectorVariant(selector, 'motion-safe', separator, message => {
-        throw container.error(message)
+  'motion-safe': generateVariantFunction(
+    ({ container, separator, modifySelectors }) => {
+      const modified = modifySelectors(({ selector }) => {
+        return buildSelectorVariant(selector, 'motion-safe', separator, message => {
+          throw container.error(message)
+        })
       })
-    })
-    const mediaQuery = postcss.atRule({
-      name: 'media',
-      params: '(prefers-reduced-motion: no-preference)',
-    })
-    mediaQuery.append(modified)
-    container.append(mediaQuery)
-  }),
-  'motion-reduce': generateVariantFunction(({ container, separator, modifySelectors }) => {
-    const modified = modifySelectors(({ selector }) => {
-      return buildSelectorVariant(selector, 'motion-reduce', separator, message => {
-        throw container.error(message)
+      const mediaQuery = postcss.atRule({
+        name: 'media',
+        params: '(prefers-reduced-motion: no-preference)',
       })
-    })
-    const mediaQuery = postcss.atRule({
-      name: 'media',
-      params: '(prefers-reduced-motion: reduce)',
-    })
-    mediaQuery.append(modified)
-    container.append(mediaQuery)
-  }),
+      mediaQuery.append(modified)
+      container.append(mediaQuery)
+    },
+    { unstable_stack: true }
+  ),
+  'motion-reduce': generateVariantFunction(
+    ({ container, separator, modifySelectors }) => {
+      const modified = modifySelectors(({ selector }) => {
+        return buildSelectorVariant(selector, 'motion-reduce', separator, message => {
+          throw container.error(message)
+        })
+      })
+      const mediaQuery = postcss.atRule({
+        name: 'media',
+        params: '(prefers-reduced-motion: reduce)',
+      })
+      mediaQuery.append(modified)
+      container.append(mediaQuery)
+    },
+    { unstable_stack: true }
+  ),
   'group-hover': generateVariantFunction(({ modifySelectors, separator }) => {
     const parser = selectorParser(selectors => {
       selectors.walkClasses(sel => {
@@ -88,9 +94,7 @@ const defaultVariantGenerators = config => ({
   even: generatePseudoClassVariant('nth-child(even)', 'even'),
 })
 
-function prependStackableVariants(atRule, variants) {
-  const stackableVariants = ['dark', 'motion-safe', 'motion-reduce']
-
+function prependStackableVariants(atRule, variants, stackableVariants) {
   if (!_.some(variants, v => stackableVariants.includes(v))) {
     return variants
   }
@@ -117,6 +121,10 @@ export default function(config, { variantGenerators: pluginVariantGenerators })
       ...pluginVariantGenerators,
     }
 
+    const stackableVariants = Object.entries(variantGenerators)
+      .filter(([_variant, { options }]) => options.unstable_stack)
+      .map(([variant]) => variant)
+
     let variantsFound = false
 
     do {
@@ -132,15 +140,15 @@ export default function(config, { variantGenerators: pluginVariantGenerators })
           responsiveParent.append(atRule)
         }
 
-        const remainingVariants = prependStackableVariants(atRule, variants)
+        const remainingVariants = prependStackableVariants(atRule, variants, stackableVariants)
 
         _.forEach(_.without(ensureIncludesDefault(remainingVariants), 'responsive'), variant => {
           if (!variantGenerators[variant]) {
             throw new Error(
               `Your config mentions the "${variant}" variant, but "${variant}" doesn't appear to be a variant. Did you forget or misconfigure a plugin that supplies that variant?`
             )
           }
-          variantGenerators[variant](atRule, config)
+          variantGenerators[variant].handler(atRule, config)
         })
 
         atRule.remove()
diff --git a/src/util/generateVariantFunction.js b/src/util/generateVariantFunction.js
@@ -12,35 +12,38 @@ const getClassNameFromSelector = useMemo(
   selector => selector
 )
 
-export default function generateVariantFunction(generator) {
-  return (container, config) => {
-    const cloned = postcss.root({ nodes: container.clone().nodes })
+export default function generateVariantFunction(generator, options = {}) {
+  return {
+    options,
+    handler: (container, config) => {
+      const cloned = postcss.root({ nodes: container.clone().nodes })
 
-    container.before(
-      _.defaultTo(
-        generator({
-          container: cloned,
-          separator: config.separator,
-          modifySelectors: modifierFunction => {
-            cloned.each(rule => {
-              if (rule.type !== 'rule') {
-                return
-              }
+      container.before(
+        _.defaultTo(
+          generator({
+            container: cloned,
+            separator: config.separator,
+            modifySelectors: modifierFunction => {
+              cloned.each(rule => {
+                if (rule.type !== 'rule') {
+                  return
+                }
 
-              rule.selectors = rule.selectors.map(selector => {
-                return modifierFunction({
-                  get className() {
-                    return getClassNameFromSelector(selector)
-                  },
-                  selector,
+                rule.selectors = rule.selectors.map(selector => {
+                  return modifierFunction({
+                    get className() {
+                      return getClassNameFromSelector(selector)
+                    },
+                    selector,
+                  })
                 })
               })
-            })
-            return cloned
-          },
-        }),
-        cloned
-      ).nodes
-    )
+              return cloned
+            },
+          }),
+          cloned
+        ).nodes
+      )
+    },
   }
 }
diff --git a/src/util/processPlugins.js b/src/util/processPlugins.js
@@ -126,8 +126,8 @@ export default function(plugins, config) {
       addBase: baseStyles => {
         pluginBaseStyles.push(wrapWithLayer(parseStyles(baseStyles), 'base'))
       },
-      addVariant: (name, generator) => {
-        pluginVariantGenerators[name] = generateVariantFunction(generator)
+      addVariant: (name, generator, options = {}) => {
+        pluginVariantGenerators[name] = generateVariantFunction(generator, options)
       },
     })
   })
