Skip to content

Improve performance of upgrade tool #18068

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Upgrade: Do not migrate declarations that look like candidates in `<style>` blocks ([#18057](https://github.com/tailwindlabs/tailwindcss/pull/18057))
- Upgrade: Do not migrate declarations that look like candidates in `<style>` blocks ([#18057](https://github.com/tailwindlabs/tailwindcss/pull/18057), [18068](https://github.com/tailwindlabs/tailwindcss/pull/18068))

## [4.1.7] - 2025-05-15

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { parseCandidate } from '../../../../tailwindcss/src/candidate'
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map'
import * as version from '../../utils/version'

const QUOTES = ['"', "'", '`']
Expand Down Expand Up @@ -44,16 +45,23 @@ export function isSafeMigration(
// Whitespace before the candidate
location.contents[location.start - 1]?.match(/\s/) &&
// A colon followed by whitespace after the candidate
location.contents.slice(location.end, location.end + 2)?.match(/^:\s/) &&
// A `<style` block is present before the candidate
location.contents.slice(0, location.start).includes('<style') &&
// `</style>` is present after the candidate
location.contents.slice(location.end).includes('</style>')
location.contents.slice(location.end, location.end + 2)?.match(/^:\s/)
) {
return false
// Compute all `<style>` ranges once and cache it for the current files
let ranges = styleBlockRanges.get(location.contents)

for (let i = 0; i < ranges.length; i += 2) {
let start = ranges[i]
let end = ranges[i + 1]

// Check if the candidate is inside a `<style>` block
if (location.start >= start && location.end <= end) {
return false
}
}
}

let [candidate] = Array.from(parseCandidate(rawCandidate, designSystem))
let [candidate] = parseCandidate(rawCandidate, designSystem)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small improvement, no need to convert to an array first...


// If we can't parse the candidate, then it's not a candidate at all. However,
// we could be dealing with legacy classes like `tw__flex` in Tailwind CSS v3
Expand All @@ -62,10 +70,10 @@ export function isSafeMigration(
// So let's only skip if we couldn't parse and we are not in Tailwind CSS v3.
//
if (!candidate && version.isGreaterThan(3)) {
return true
return false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit of a dumb dumb here. When we know that a candidate doesn't parse, and we know that we are in v4 and up, then we know that we can't migrate the candidate anyway so we can bail.

That makes it an unsafe migration (which skips all the migrations for this candidate). We were returning true which means that we were migrating the candidate. This still worked because eventually the migrations were no-ops because they all bailed.

But let's not do that work in the first place, yeah?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change alone makes the upgrade tool on Tailwind UI's product folder go from ~12s to ~9s

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and that's because this branch was hit about 1 041 458 times in that codebase.

}

// Parsed a candidate succesfully, verify if it's a valid candidate
// Parsed a candidate successfully, verify if it's a valid candidate
else if (candidate) {
// When we have variants, we can assume that the candidate is safe to migrate
// because that requires colons.
Expand Down Expand Up @@ -168,3 +176,30 @@ export function isSafeMigration(

return true
}

// Assumptions:
// - All `<style` tags appear before the next `</style>` tag
// - All `<style` tags are closed with `</style>`
// - No nested `<style>` tags
const styleBlockRanges = new DefaultMap((source: string) => {
let ranges: number[] = []
let offset = 0

while (true) {
let startTag = source.indexOf('<style', offset)
if (startTag === -1) return ranges

// Ensure the style looks like:
// - `<style>` (closed)
// - `<style …>` (with attributes)
if (!source[startTag + 6].match(/[>\s]/)) return ranges

offset = startTag + 1
Comment on lines +192 to +197
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will stop scanning as soon as it sees a tag that starts with <style but isn't <style>. e.g. <styleless>. How about this?

Suggested change
// Ensure the style looks like:
// - `<style>` (closed)
// - `<style …>` (with attributes)
if (!source[startTag + 6].match(/[>\s]/)) return ranges
offset = startTag + 1
offset = startTag + 1
// Ensure the style looks like:
// - `<style>` (closed)
// - `<style …>` (with attributes)
if (!source[startTag + 6].match(/[>\s]/)) continue


let endTag = source.indexOf('</style>', offset)
if (endTag === -1) return ranges
offset = endTag + 1

ranges.push(startTag, endTag)
}
})