From 4e40891c3943f40d786cb5475ad1bae94c2c7339 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 29 Sep 2021 11:57:42 +0200 Subject: [PATCH 1/2] detect ambiguity in arbitrary values --- src/lib/generateRules.js | 82 ++++++++++++++++++++++++++++++++-- tests/arbitrary-values.test.js | 12 +++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/lib/generateRules.js b/src/lib/generateRules.js index 265e2482e0e8..852185091b37 100644 --- a/src/lib/generateRules.js +++ b/src/lib/generateRules.js @@ -4,6 +4,7 @@ import parseObjectStyles from '../util/parseObjectStyles' import isPlainObject from '../util/isPlainObject' import prefixSelector from '../util/prefixSelector' import { updateAllClasses } from '../util/pluginUtils' +import log from '../util/log' let classNameParser = selectorParser((selectors) => { return selectors.first.filter(({ type }) => type === 'class').pop().value @@ -225,15 +226,19 @@ function* resolveMatches(candidate, context) { for (let matchedPlugins of resolveMatchedPlugins(classCandidate, context)) { let matches = [] + let typesByMatches = new Map() + let [plugins, modifier] = matchedPlugins let isOnlyPlugin = plugins.length === 1 for (let [sort, plugin] of plugins) { + let matchesPerPlugin = [] + if (typeof plugin === 'function') { for (let ruleSet of [].concat(plugin(modifier, { isOnlyPlugin }))) { let [rules, options] = parseRules(ruleSet, context.postCssNodeCache) for (let rule of rules) { - matches.push([{ ...sort, options: { ...sort.options, ...options } }, rule]) + matchesPerPlugin.push([{ ...sort, options: { ...sort.options, ...options } }, rule]) } } } @@ -242,12 +247,79 @@ function* resolveMatches(candidate, context) { let ruleSet = plugin let [rules, options] = parseRules(ruleSet, context.postCssNodeCache) for (let rule of rules) { - matches.push([{ ...sort, options: { ...sort.options, ...options } }, rule]) + matchesPerPlugin.push([{ ...sort, options: { ...sort.options, ...options } }, rule]) } } + + if (matchesPerPlugin.length > 0) { + typesByMatches.set(matchesPerPlugin, sort.options?.type) + matches.push(matchesPerPlugin) + } } - matches = applyPrefix(matches, context) + // Only keep the result of the very first plugin if we are dealing with + // arbitrary values, to protect against ambiguity. + if (isArbitraryValue(modifier) && matches.length > 1) { + let typesPerPlugin = matches.map((match) => new Set([...(typesByMatches.get(match) ?? [])])) + + // Remove duplicates, so that we can detect proper unique types for each plugin. + for (let pluginTypes of typesPerPlugin) { + for (let type of pluginTypes) { + let removeFromOwnGroup = false + + for (let otherGroup of typesPerPlugin) { + if (pluginTypes === otherGroup) continue + + if (otherGroup.has(type)) { + otherGroup.delete(type) + removeFromOwnGroup = true + } + } + + if (removeFromOwnGroup) pluginTypes.delete(type) + } + } + + let messages = [] + + for (let [idx, group] of typesPerPlugin.entries()) { + for (let type of group) { + let rules = matches[idx] + .map(([, rule]) => rule) + .flat() + .map((rule) => + rule + .toString() + .split('\n') + .slice(1, -1) // Remove selector and closing '}' + .map((line) => line.trim()) + .map((x) => ` ${x}`) // Re-indent + .join('\n') + ) + .join('\n\n') + + messages.push( + ` - Replace "${candidate}" with "${candidate.replace( + '[', + `[${type}:` + )}" for:\n${rules}\n` + ) + break + } + } + + log.warn([ + // TODO: Improve this warning message + `We found ${matches.length} plugins that generated output for "${candidate}".`, + // TODO: Update URL + 'You can be explicit by introducing typehints: https://tailwindcss.com/docs/just-in-time-mode#ambiguous-values', + '', + ...messages, + ]) + continue + } + + matches = applyPrefix(matches.flat(), context) if (important) { matches = applyImportant(matches, context) @@ -317,4 +389,8 @@ function generateRules(candidates, context) { }) } +function isArbitraryValue(input) { + return input.startsWith('[') && input.endsWith(']') +} + export { resolveMatches, generateRules } diff --git a/tests/arbitrary-values.test.js b/tests/arbitrary-values.test.js index f264faa0c9e4..cde5846382d6 100644 --- a/tests/arbitrary-values.test.js +++ b/tests/arbitrary-values.test.js @@ -197,3 +197,15 @@ it('should not convert escaped underscores with spaces', () => { `) }) }) + +it('should warn and not generate if arbitrary values are ambigu', () => { + // If we don't protect against this, then `bg-[200px_100px]` would both + // generate the background-size as well as the background-position utilities. + let config = { + content: [{ raw: html`
` }], + } + + return run('@tailwind utilities', config).then((result) => { + return expect(result.css).toMatchFormattedCss(css``) + }) +}) From cf9297a511564066a03165c54e1690b82b57abf6 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 29 Sep 2021 13:31:43 +0200 Subject: [PATCH 2/2] update warning message --- src/lib/generateRules.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/generateRules.js b/src/lib/generateRules.js index 852185091b37..13ec474803cb 100644 --- a/src/lib/generateRules.js +++ b/src/lib/generateRules.js @@ -309,12 +309,13 @@ function* resolveMatches(candidate, context) { } log.warn([ - // TODO: Improve this warning message - `We found ${matches.length} plugins that generated output for "${candidate}".`, // TODO: Update URL - 'You can be explicit by introducing typehints: https://tailwindcss.com/docs/just-in-time-mode#ambiguous-values', + `The class "${candidate}" is ambiguous and matches multiple utilities. Use a type hint (https://tailwindcss.com/docs/just-in-time-mode#ambiguous-values) to fix this.`, '', ...messages, + `If this is just part of your content and not a class, replace it with "${candidate + .replace('[', '[') + .replace(']', ']')}" to silence this warning.`, ]) continue }