Skip to content

Commit

Permalink
Merge pull request #300 from dcastil/breaking-bugfix/297/fix-backgrou…
Browse files Browse the repository at this point in the history
…nd-image-and-background-position-being-merged-incorrectly

Fix `background-image` and `background-position` being merged incorrectly
  • Loading branch information
dcastil authored Aug 27, 2023
2 parents 052e89b + 742420a commit 31dcd29
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 29 deletions.
4 changes: 2 additions & 2 deletions docs/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ interface Validators {
isTshirtSize(value: string): boolean
isArbitrarySize(value: string): boolean
isArbitraryPosition(value: string): boolean
isArbitraryUrl(value: string): boolean
isArbitraryImage(value: string): boolean
isArbitraryNumber(value: string): boolean
isArbitraryShadow(value: string): boolean
isAny(value: string): boolean
Expand All @@ -339,7 +339,7 @@ A brief summary for each validator:
- `isTshirtSize`checks whether class part is a T-shirt size (`sm`, `xl`), optionally with a preceding number (`2xl`).
- `isArbitrarySize` checks whether class part is an arbitrary value which starts with `size:` (`[size:200px_100px]`) which is necessary for background-size classNames.
- `isArbitraryPosition` checks whether class part is an arbitrary value which starts with `position:` (`[position:200px_100px]`) which is necessary for background-position classNames.
- `isArbitraryUrl` checks whether class part is an arbitrary value which starts with `url:` or `url(` (`[url('/path-to-image.png')]`, `url:var(--maybe-a-url-at-runtime)]`) which is necessary for background-image classNames.
- `isArbitraryImage` checks whether class part is an arbitrary value which is an iamge, e.g. by starting with `image:`, `url:`, `linear-gradient(` or `url(` (`[url('/path-to-image.png')]`, `image:var(--maybe-an-image-at-runtime)]`) which is necessary for background-image classNames.
- `isArbitraryShadow` checks whether class part is an arbitrary value which starts with the same pattern as a shadow value (`[0_35px_60px_-15px_rgba(0,0,0,0.3)]`), namely with two lengths separated by a underscore.
- `isAny` always returns true. Be careful with this validator as it might match unwanted classes. I use it primarily to match colors or when I'm certain there are no other class groups in a namespace.
Expand Down
18 changes: 18 additions & 0 deletions docs/changelog/v1-to-v2-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ By exports:
- `validators`
- [`isLength`: Does not check for arbitrary values anymore](#validatorsislength-does-not-check-for-arbitrary-values-anymore)
- [`isInteger`: Does not check for arbitrary values anymore](#validatorsisinteger-does-not-check-for-arbitrary-values-anymore)
- [`isArbitraryUrl`: Renamed](#validatorsisarbitraryurl-renamed)
- [`isArbitraryWeight`: Removed](#validatorsisarbitraryweight-removed)
- `createTailwindMerge`
- [Mandatory elements added](#createtailwindmerge-mandatory-elements-added)
Expand Down Expand Up @@ -301,6 +302,23 @@ If those classes use arbitrary values but there is only a single class group tha

Otherwise, proceed as shown in the minimal upgrade.

### `validators.isArbitraryUrl`: Renamed

Related: [#300](https://github.com/dcastil/tailwind-merge/pull/300)

`isArbitraryUrl` was used to detect arbitrary `background-image` values. However, the `background-image` CSS property supports more than just URLs, so the functionality of the validator was expanded to also detect values like `image:var(--maybe-an-image-at-runtime)]` or `linear-gradient(rgba(0,0,255,0.5),rgba(255,255,0,0.5))` and therefore renamed as well.

#### Upgrade

Replace all uses of `validators.isArbitraryUrl` with `validators.isArbitraryImage`.

```diff
import { validators } from 'tailwind-merge'

- validators.isArbitraryUrl
+ validators.isArbitraryImage
```

### `validators.isArbitraryWeight`: Removed

Related: [#288](https://github.com/dcastil/tailwind-merge/pull/288)
Expand Down
4 changes: 2 additions & 2 deletions src/lib/default-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { fromTheme } from './from-theme'
import { Config, DefaultClassGroupIds, DefaultThemeGroupIds } from './types'
import {
isAny,
isArbitraryImage,
isArbitraryLength,
isArbitraryNumber,
isArbitraryPosition,
isArbitraryShadow,
isArbitrarySize,
isArbitraryUrl,
isArbitraryValue,
isInteger,
isLength,
Expand Down Expand Up @@ -903,7 +903,7 @@ export function getDefaultConfig() {
bg: [
'none',
{ 'gradient-to': ['t', 'tr', 'r', 'br', 'b', 'bl', 'l', 'tl'] },
isArbitraryUrl,
isArbitraryImage,
],
},
],
Expand Down
36 changes: 23 additions & 13 deletions src/lib/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const lengthUnitRegex =
/\d+(%|px|r?em|[sdl]?v([hwib]|min|max)|pt|pc|in|cm|mm|cap|ch|ex|r?lh|cq(w|h|i|b|min|max))|\b(calc|min|max|clamp)\(.+\)|^0$/
// Shadow always begins with x and y offset separated by underscore
const shadowRegex = /^-?((\d+)?\.?(\d+)[a-z]+|0)_-?((\d+)?\.?(\d+)[a-z]+|0)/
const imageRegex =
/^(url|image|image-set|cross-fade|element|(repeating-)?(linear|radial|conic)-gradient)\(.+\)$/

export function isLength(value: string) {
return isNumber(value) || stringLengths.has(value) || fractionRegex.test(value)
Expand Down Expand Up @@ -39,16 +41,20 @@ export function isTshirtSize(value: string) {
return tshirtUnitRegex.test(value)
}

const sizeLabels = new Set(['length', 'size', 'percentage'])

export function isArbitrarySize(value: string) {
return getIsArbitraryValue(value, 'size', isNever)
return getIsArbitraryValue(value, sizeLabels, isNever)
}

export function isArbitraryPosition(value: string) {
return getIsArbitraryValue(value, 'position', isNever)
}

export function isArbitraryUrl(value: string) {
return getIsArbitraryValue(value, 'url', isUrl)
const imageLabels = new Set(['image', 'url'])

export function isArbitraryImage(value: string) {
return getIsArbitraryValue(value, imageLabels, isImage)
}

export function isArbitraryShadow(value: string) {
Expand All @@ -59,16 +65,16 @@ export function isAny() {
return true
}

function isLengthOnly(value: string) {
return lengthUnitRegex.test(value)
}

function getIsArbitraryValue(value: string, label: string, testValue: (value: string) => boolean) {
function getIsArbitraryValue(
value: string,
label: string | Set<string>,
testValue: (value: string) => boolean,
) {
const result = arbitraryValueRegex.exec(value)

if (result) {
if (result[1]) {
return result[1] === label
return typeof label === 'string' ? result[1] === label : label.has(result[1])
}

return testValue(result[2]!)
Expand All @@ -77,14 +83,18 @@ function getIsArbitraryValue(value: string, label: string, testValue: (value: st
return false
}

function isNever() {
return false
function isLengthOnly(value: string) {
return lengthUnitRegex.test(value)
}

function isUrl(value: string) {
return value.startsWith('url(')
function isNever() {
return false
}

function isShadow(value: string) {
return shadowRegex.test(value)
}

function isImage(value: string) {
return imageRegex.test(value)
}
8 changes: 8 additions & 0 deletions tests/arbitrary-values.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,12 @@ test('handles ambiguous arbitrary values correctly', () => {
expect(twMerge('text-2xl text-[calc(theme(fontSize.4xl)/1.125)]')).toBe(
'text-[calc(theme(fontSize.4xl)/1.125)]',
)
expect(twMerge('bg-cover bg-[percentage:30%] bg-[length:200px_100px]')).toBe(
'bg-[length:200px_100px]',
)
expect(
twMerge(
'bg-none bg-[url(.)] bg-[image:.] bg-[url:.] bg-[linear-gradient(.)] bg-gradient-to-r',
),
).toBe('bg-gradient-to-r')
})
4 changes: 2 additions & 2 deletions tests/public-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ test('has correct export types', () => {
isArbitraryPosition: expect.any(Function),
isArbitraryShadow: expect.any(Function),
isArbitrarySize: expect.any(Function),
isArbitraryUrl: expect.any(Function),
isArbitraryImage: expect.any(Function),
isArbitraryValue: expect.any(Function),
isInteger: expect.any(Function),
isLength: expect.any(Function),
Expand Down Expand Up @@ -152,7 +152,7 @@ test('validators have correct inputs and outputs', () => {
expect(validators.isTshirtSize('')).toEqual(expect.any(Boolean))
expect(validators.isArbitrarySize('')).toEqual(expect.any(Boolean))
expect(validators.isArbitraryPosition('')).toEqual(expect.any(Boolean))
expect(validators.isArbitraryUrl('')).toEqual(expect.any(Boolean))
expect(validators.isArbitraryImage('')).toEqual(expect.any(Boolean))
expect(validators.isArbitraryNumber('')).toEqual(expect.any(Boolean))
expect(validators.isArbitraryShadow('')).toEqual(expect.any(Boolean))
})
Expand Down
25 changes: 15 additions & 10 deletions tests/validators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {
isTshirtSize,
isArbitrarySize,
isArbitraryPosition,
isArbitraryUrl,
isArbitraryImage,
isArbitraryNumber,
isArbitraryShadow,
} = validators
Expand Down Expand Up @@ -120,6 +120,8 @@ test('isTshirtSize', () => {
test('isArbitrarySize', () => {
expect(isArbitrarySize('[size:2px]')).toBe(true)
expect(isArbitrarySize('[size:bla]')).toBe(true)
expect(isArbitrarySize('[length:bla]')).toBe(true)
expect(isArbitrarySize('[percentage:bla]')).toBe(true)

expect(isArbitrarySize('[2px]')).toBe(false)
expect(isArbitrarySize('[bla]')).toBe(false)
Expand All @@ -135,15 +137,18 @@ test('isArbitraryPosition', () => {
expect(isArbitraryPosition('position:2px')).toBe(false)
})

test('isArbitraryUrl', () => {
expect(isArbitraryUrl('[url:var(--my-url)]')).toBe(true)
expect(isArbitraryUrl('[url(something)]')).toBe(true)
expect(isArbitraryUrl('[url:bla]')).toBe(true)

expect(isArbitraryUrl('[var(--my-url)]')).toBe(false)
expect(isArbitraryUrl('[bla]')).toBe(false)
expect(isArbitraryUrl('url:2px')).toBe(false)
expect(isArbitraryUrl('url(2px)')).toBe(false)
test('isArbitraryImage', () => {
expect(isArbitraryImage('[url:var(--my-url)]')).toBe(true)
expect(isArbitraryImage('[url(something)]')).toBe(true)
expect(isArbitraryImage('[url:bla]')).toBe(true)
expect(isArbitraryImage('[image:bla]')).toBe(true)
expect(isArbitraryImage('[linear-gradient(something)]')).toBe(true)
expect(isArbitraryImage('[repeating-conic-gradient(something)]')).toBe(true)

expect(isArbitraryImage('[var(--my-url)]')).toBe(false)
expect(isArbitraryImage('[bla]')).toBe(false)
expect(isArbitraryImage('url:2px')).toBe(false)
expect(isArbitraryImage('url(2px)')).toBe(false)
})

test('isArbitraryNumber', () => {
Expand Down

0 comments on commit 31dcd29

Please sign in to comment.