-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Add setup for template migrations #14502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
907b5ef to
7ce3641
Compare
7ce3641 to
da60fb8
Compare
| let candidates = await extractCandidates(designSystem, contents) | ||
|
|
||
| // Sort candidates by starting position desc | ||
| candidates.sort((a, z) => z.start - a.start) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is smart, I was wondering how you were planning to keep track of the position of candidates changing as you insert replacements 🧠
RobinMalfait
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sweet! Approved, but a few suggestions / questions.
| } | ||
| `, | ||
| 'src/index.html': html` | ||
| <h1>🤠👋</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Howdy!
packages/@tailwindcss-upgrade/src/template/codemods/migrate-important.ts
Outdated
Show resolved
Hide resolved
| * | ||
| * - `underline` | ||
| * - `flex` | ||
| * - `box-border` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it matters, but what was the idea behind this change? Just an example with a special character like -?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha flex is not a static utility but a functional one now, so this comment is wrong. I put it as a test case and got confused for a second why it didn't work while I implemented my initial version of the printer and then noticed oh it's cause it's functional. I thought I update it just so it's more correct 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah right, forgot about that. Good catch!
Co-authored-by: Robin Malfait <[email protected]>
Co-authored-by: Jordan Pittman <[email protected]>
This PR adds the initial setup and a first codemod for the template migrations. These are a new set of migrations that operate on files defined in the Tailwind v3 config as part of the
contentoption (so your HTML, JavaScript, TSX files etc.).The migration for this is integrated in the new
@tailwindcss/upgradepackage and will require pointing the migration to an input JavaScript config file, like this:The idea of template migrations is to apply breaking changes from the v3 to v4 migration within your template files.
Migrating !important syntax
The first migration that I’m adding with this PR is to ensure we use the v4 important syntax that has the exclamation mark at the end of the utility.
For example, this:
Will now turn into:
Architecture considerations
Implementation wise, we make use of Oxide to scan the content files fast and efficiently. By relying on the same scanner als Tailwind v4, we guarantee that all candidates that are part of the v4 output will have gone through a migration.
Migrations itself operate on the abstract
Candidatetype, similar to the type we use in the v4 codebase. It will parse the candidate into its parts so they can easily be introspected/modified. Migrations are typed as:nullshould be returned if theCandidatedoes not need a migration.We currently use the v4
parseCandidatefunction to get an abstract definition of the candidate rule that we can operate on. This will likely need to change in the future as we need to forkparseCandidatefor v3 specific syntax.Additionally, we're inlining a
printCandidatefunction that can stringify the abstractCandidatetype. It is not guaranteed that this is an identity function since some information can be lost during the parse step. This is not a problem though, because migrations will only run selectively and if none of the selectors trigger, the candidates are not updated. h/t to @RobinMalfait for providing the printer.So the overall flow of a migration looks like this:
contentfilescontentfilesCandidateabstract type.Candidateback into the originalcontentfile.