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

* 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>
2020-09-14 09:12:36 -04:00 · 2020-09-14 09:12:36 -04:00 · 32eac19858
commit 32eac19858
parent 2d8cf7fa0d
7 changed files with 144 additions and 86 deletions
--- 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": [
--- 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

--- 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 {
--- 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
-    }
-
-    if (config('dark') === 'class') {
-      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
+      }

-      modified.walkRules(rule => {
-        rule.selectors = rule.selectors.map(selector => {
-          return `${prefix('.dark')} ${selector}`
+      if (config('dark') === 'class') {
+        const modified = modifySelectors(({ selector }) => {
+          return buildSelectorVariant(selector, 'dark', separator, message => {
+            throw container.error(message)
+          })
        })
-      })

-      return modified
-    }
+        modified.walkRules(rule => {
+          rule.selectors = rule.selectors.map(selector => {
+            return `${prefix('.dark')} ${selector}`
+          })
+        })

-    throw new Error("The `dark` config option must be either 'media' or 'class'.")
-  })
+        return modified
+      }
+
+      throw new Error("The `dark` config option must be either 'media' or 'class'.")
+    },
+    { unstable_stack: true }
+  )
 }
--- 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,7 +140,7 @@ 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]) {
@ -140,7 +148,7 @@ export default function(config, { variantGenerators: pluginVariantGenerators })
              `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()
--- 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
+      )
+    },
  }
 }
--- 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)
      },
    })
  })