From 078e0f8e1b975f7b86a4ccf2ec87647a5bc5d74c Mon Sep 17 00:00:00 2001 From: Adam Wathan Date: Mon, 14 Sep 2020 09:12:36 -0400 Subject: [PATCH] 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> --- .eslintrc.json | 1 + CHANGELOG.md | 4 +- __tests__/darkMode.test.js | 40 +++++++++++++++++ src/flagged/darkModeVariantPlugin.js | 58 ++++++++++++------------ src/lib/substituteVariantsAtRules.js | 66 ++++++++++++++++------------ src/util/generateVariantFunction.js | 55 ++++++++++++----------- src/util/processPlugins.js | 4 +- 7 files changed, 143 insertions(+), 85 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 93807e75b3bf..42a6e3c88373 100644 --- 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 index 1b4fc0bcaaa7..e59c56e362ab 100644 --- 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 index 477c6d801683..eda74a4967ad 100644 --- 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 index 9550e2c7a401..fb9606702634 100644 --- 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 index 2b2bdf78d05d..3f3b91c75c0f 100644 --- 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() diff --git a/src/util/generateVariantFunction.js b/src/util/generateVariantFunction.js index 8de4e4867a7f..83e91e16cee3 100644 --- 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 index 066f5764826b..5e02a27215ef 100644 --- 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) }, }) })