docs: add migrate-styled-components-to-vanilla-extract skill - #1774
Conversation
Clarify that the Vite plugin remains required for .css.ts transforms, while disableRuntimeStyles is an optional jsdom/happy-dom opt-in when tests do not need real CSS injection.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
✅ E2E Tests🟢 25 passed • 🟡 1 flaky • view full report • view run Studio: https://plugins-e2e-test-studio-64sbv49xg.sanity.dev Datasets: |
Coverage Report
File CoverageNo changed files found. |
…time-styles-docs-9315 # Conflicts: # AGENTS.md
Step-by-step migration procedure distilled from PR #1417 (@sanity/google-maps-input) and PR #1450 (sanity-plugin-workflow), covering inventory, code patterns, tsdown config, dependency changes (including the styled-components devDep peer-alignment pitfall), Vitest wiring, snapshot updates, verification, and changesets.
Point sanity-plugin-best-practices and plugin-transfer at the new step-by-step skill, and update the styling reference's post-migration dependency guidance to match merged practice: keep the styled-components catalog devDependency while the plugin depends on @sanity/ui.
Adds Shape C to the migration skill: per-instance/theme-driven values bridged through createVar + assignInlineVars, including migrating computed inline style objects, with the Checkboard example.
Link the in-repo 'Disabling runtime styles in tests' section instead of the external doc, add @sanity/color-input as an in-repo reference implementation, and soften the styled-components devDep guidance to verify-lockfile-alignment now that the @sanity/styled-components override is gone and color-input merged without the devDep.
68a1789 to
394e45a
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new internal agent skill documenting the repeatable procedure for migrating a single plugin from styled-components to vanilla-extract, and updates existing skill docs to cross-link to this new procedure (while keeping the underlying styling patterns in the existing styling reference).
Changes:
- Introduces
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.mdwith an end-to-end migration workflow (inventory → code shapes → tsdown config → deps → Vitest wiring → verification → changeset). - Updates
sanity-plugin-best-practicesdocs to point to the new skill for the procedure and keepsreferences/styling.mdfocused on patterns. - Updates
plugin-transferguidance to route follow-up styling migrations to the new skill (and reiterates “don’t migrate styling during transfers”).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .agents/skills/sanity-plugin-best-practices/SKILL.md | Adds a cross-link to the new migration procedure skill. |
| .agents/skills/sanity-plugin-best-practices/references/styling.md | Clarifies “migrating off styled-components” section and links to the new procedure skill. |
| .agents/skills/plugin-transfer/SKILL.md | Points follow-up styling migrations (post-transfer) to the new skill. |
| .agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md | New skill: step-by-step migration procedure docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR links rot over time and can point at patterns we've since moved away from. Cite the in-repo reference plugins (@sanity/google-maps-input, sanity-plugin-workflow, @sanity/color-input) directly instead.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:133
- The example JSX snippet has a leading semicolon (
;<div ...>), which makes the sample confusing and is not a pattern we want to encourage in docs. It reads like a typo rather than intentional ASI protection here.
;<div
Keep the skill reusable across repos: ground rules like one-plugin-per-PR, never-during-transfer, and the reference-implementation plugin list live in AGENTS.md. The skill keeps the portable procedure and points at AGENTS.md for monorepo-specific policy.
Use the repo-agnostic 'workspace' term for the unit being migrated so the skill reads naturally outside this plugins monorepo. Literal package.json, the package-exports test, the Vite plugin, and the AGENTS.md policy pointer keep their names.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:213
- The note about a “host app or studio” registering the Vite plugin globally is ambiguous in the Vitest section and could be read as permission to skip adding
vanillaExtractPlugin()to the package’svitest.config.ts(which would break.css.tscompilation in package tests). Consider explicitly calling out the test studio file and clarifying that the package’s Vitest config still needs the plugin.
Use `@sanity/vanilla-extract-vite-plugin` (faster drop-in for the upstream
`@vanilla-extract/vite-plugin`). If a host app or studio already registers the Vite plugin
globally, you don't need to touch that file for the workspace under migration.
Spreading {...props} then a fixed className drops a caller-provided
className. Pull className out of props and merge with clsx(localClass,
className) in the Shape A example and the styling reference's
encapsulation wrappers, noting any classnames helper or template literal
works.
…-components-skill-91e1 # Conflicts: # .agents/skills/sanity-plugin-best-practices/references/styling.md # AGENTS.md
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:217
- This paragraph suggests you can skip updating the package’s
vitest.config.tsif a host app/studio registers the Vite plugin globally, but Vitest does not inherit the test-studio Vite config. The plugin is still required in the package’s ownvitest.config.ts(e.g. for the package-exports test importing.css.ts).
// ...existing test config
})
**.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:128**
* The TSX snippet has a stray leading semicolon (`;<div`), which makes the example look like a typo and isn’t needed here (it’s not preceded by an expression that would require ASI protection).
This issue also appears on line 215 of the same file.
function Checkboard({borderRadius, background}: {borderRadius?: string; background?: string}) {
</details>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (6)
.agents/skills/sanity-plugin-best-practices/references/styling.md:458
- This bullet references
clsx(...), which isn’t a dependency used/declared by plugin packages in this repo. Updating it to show a dependency-free merge pattern keeps the guidance copy/paste-safe.
- **Merge, don't clobber, `className`.** A fixed `className={errorCode}` after `{...props}` silently
drops a `className` a caller passes in. Pull `className` out of props and merge it —
`clsx(errorCode, className)` (any classnames helper, or a `` `${errorCode} ${className ?? ''}` ``
template literal) — so the wrapper's class always applies while still honoring the caller's.
.agents/skills/sanity-plugin-best-practices/references/styling.md:466
- This snippet still uses
clsx(...), but the repo doesn’t currently standardize on (or declare)clsxas a plugin dependency. Using a simple string join keeps the example dependency-free and consistent with the guidance above.
export function Root({className, ...props}: ComponentProps<typeof Flex>) {
return <Flex {...props} className={clsx(root, className)} />
}
.agents/skills/sanity-plugin-best-practices/references/styling.md:482
- Same as above: this example uses
clsx(...), which plugin packages here don’t declare as a dependency. A dependency-free join avoids implying authors should add an extra package just to merge classNames.
<Card
{...props}
className={clsx(resultContainer, isInvalid && resultContainerInvalid, className)}
/>
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:89
- This paragraph recommends
clsx(...)/classnames, but those aren’t currently standard/declared dependencies for plugin packages here. Consider describing a dependency-free merge pattern to avoid implying consumers should add extra deps.
`className={floatingCard}` after `{...props}` silently drops a `className` a caller passes in. Pull
`className` out of props and merge it with `clsx(floatingCard, className)` (any classnames helper —
`clsx`, `classnames` — or a `` `${floatingCard} ${className ?? ''}` `` template literal works) so the
wrapper's class is always applied while still honoring the caller's. See
.agents/skills/sanity-plugin-best-practices/references/styling.md:444
- These docs now use
clsx, but none of the plugin packages in this repo declareclsxas a dependency. Copying this snippet into a plugin will fail to compile unless the author also adds a new dependency. Consider using a dependency-free className merge in the example (or explicitly calling out the dependency requirement).
This issue also appears in the following locations of the same file:
- line 455
- line 464
- line 479
import {Code} from '@sanity/ui'
import {clsx} from 'clsx'
import {type ComponentProps} from 'react'
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:81
- This example imports and uses
clsx, but plugin packages in this repo don’t declareclsxas a dependency. Using a dependency-free className merge keeps the snippet copy/paste-safe.
This issue also appears on line 86 of the same file.
import {Card} from '@sanity/ui'
import {clsx} from 'clsx'
import type {ComponentProps} from 'react'
…-components-skill-91e1
Wrapper className merges are string joins (plus optional falsy conditionals), so import clsx from 'clsx/lite' instead of the full clsx entry. Reserve full clsx for the object/array API.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (6)
.agents/skills/sanity-plugin-best-practices/references/styling.md:466
- This snippet uses
clsx(...), but the file no longer needs to introduce aclsxdependency for the docs example. Use a simple string merge so the wrapper preserves any caller-providedclassName.
```tsx
import {clsx} from 'clsx/lite'
.agents/skills/sanity-plugin-best-practices/references/styling.md:482
- This example still uses
clsx(...). Since the docs are already describing class merging, it’s clearer to show a dependency-free pattern (e.g. join/filter) that readers can paste without addingclsx.
isInvalid,
className,
...props
}: ComponentProps<typeof Card> & {isInvalid: boolean}) {
.agents/skills/sanity-plugin-best-practices/references/styling.md:458
- The guidance here uses a JS template literal inside Markdown inline code. Using a dependency-free join/filter example avoids nested-backtick rendering issues and keeps the docs copy/pasteable without adding
clsx.
- **Merge, don't clobber, `className`.** A fixed `className={errorCode}` after `{...props}` silently
drops a `className` a caller passes in. Pull `className` out of props and merge it —
`clsx(errorCode, className)`. Prefer `import {clsx} from 'clsx/lite'` when only joining strings
(the usual case); reach for full `clsx` only if you need the object/array API. A
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:89
- The prose currently names
clsx/classnameshelpers. Since the docs can demonstrateclassNamemerging without implying a new dependency, consider pointing at the same join/filter pattern used in the snippet.
Type the wrapper with `ComponentProps<typeof Primitive>` (or `ComponentProps<'div'>`) and never use
`forwardRef` — `ref` is a regular prop on React 19. **Merge, don't clobber:** a fixed
`className={floatingCard}` after `{...props}` silently drops a `className` a caller passes in. Pull
`className` out of props and merge it with `clsx(floatingCard, className)` — prefer
`import {clsx} from 'clsx/lite'` when only joining strings (the usual case); reach for full `clsx`
only if you need the object/array API. A `` `${floatingCard} ${className ?? ''}` `` template literal
.agents/skills/sanity-plugin-best-practices/references/styling.md:449
- The docs example uses
import {clsx} from 'clsx', butclsxis typically a default export and this repo doesn’t otherwise use it. To keep the snippet copy/pasteable without adding a new dependency, mergeclassNamewith a simple template expression instead.
This issue also appears in the following locations of the same file:
- line 455
- line 464
- line 479
import {Code} from '@sanity/ui'
import {clsx} from 'clsx/lite'
import {type ComponentProps} from 'react'
import {errorCode} from './QueryErrorDialog.css'
export function ErrorCode({className, ...props}: ComponentProps<typeof Code>) {
return <Code {...props} className={clsx(errorCode, className)} />
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:81
- This example imports/uses
clsxjust to mergeclassName. Sinceclsxisn’t otherwise used in the repo and is typically a default export, the snippet is easier to copy/paste if it uses a dependency-free join/filter merge instead.
This issue also appears on line 84 of the same file.
import {Card} from '@sanity/ui'
import {clsx} from 'clsx/lite'
import type {ComponentProps} from 'react'
import {floatingCard} from './FloatingCard.css'
function StyledFloatingCard({className, ...props}: ComponentProps<typeof Card>) {
return <Card {...props} className={clsx(floatingCard, className)} />
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (6)
.agents/skills/sanity-plugin-best-practices/references/styling.md:459
- This section recommends
clsx/lite, butclsxisn’t used or declared as a dependency in this repo. To keep the guidance immediately usable, it’s better to describe a dependency-freeclassNamemerge (template literal) here.
- **Merge, don't clobber, `className`.** A fixed `className={errorCode}` after `{...props}` silently
drops a `className` a caller passes in. Pull `className` out of props and merge it —
`clsx(errorCode, className)`. Prefer `import {clsx} from 'clsx/lite'` when only joining strings
(the usual case); reach for full `clsx` only if you need the object/array API. A
`` `${errorCode} ${className ?? ''}` `` template literal also works.
.agents/skills/sanity-plugin-best-practices/references/styling.md:469
- This example also imports
clsx/lite, which isn’t declared in the repo. Consider using the same dependency-free template-literal merge pattern as elsewhere in the guide.
import {clsx} from 'clsx/lite'
export function Root({className, ...props}: ComponentProps<typeof Flex>) {
return <Flex {...props} className={clsx(root, className)} />
}
.agents/skills/sanity-plugin-best-practices/references/styling.md:487
- Same concern here:
clsx/liteisn’t a declared dependency in this repo, so this example isn’t directly copy/pasteable. Consider rewriting the conditional class merge withoutclsx.
import {clsx} from 'clsx/lite'
function ResultContainer({
isInvalid,
className,
...props
}: ComponentProps<typeof Card> & {isInvalid: boolean}) {
return (
<Card
{...props}
className={clsx(resultContainer, isInvalid && resultContainerInvalid, className)}
/>
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:90
- This paragraph recommends
clsx/lite, but the repo doesn’t currently includeclsx. To avoid encouraging migrations to add extra deps, consider removing theclsxguidance and keeping the template-literal merge option as the primary recommendation.
Type the wrapper with `ComponentProps<typeof Primitive>` (or `ComponentProps<'div'>`) and never use
`forwardRef` — `ref` is a regular prop on React 19. **Merge, don't clobber:** a fixed
`className={floatingCard}` after `{...props}` silently drops a `className` a caller passes in. Pull
`className` out of props and merge it with `clsx(floatingCard, className)` — prefer
`import {clsx} from 'clsx/lite'` when only joining strings (the usual case); reach for full `clsx`
only if you need the object/array API. A `` `${floatingCard} ${className ?? ''}` `` template literal
also works. See
.agents/skills/sanity-plugin-best-practices/references/styling.md:446
- The example uses
clsx/lite, butclsxis not currently a dependency anywhere in this repo (nopackage.jsonincludes it). As written, this snippet won’t be copy/pasteable without adding a new dependency. Consider using a template literal (as already mentioned later in the doc) to mergeclassNamewithout requiringclsx.
This issue also appears in the following locations of the same file:
- line 455
- line 465
- line 476
import {Code} from '@sanity/ui'
import {clsx} from 'clsx/lite'
import {type ComponentProps} from 'react'
import {errorCode} from './QueryErrorDialog.css'
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md:81
- The Shape A example uses
clsx/lite, butclsxis not currently declared in anypackage.jsonin this repo. Since this skill is meant to be copy/pasteable, consider using a dependency-freeclassNamemerge (template literal) or explicitly note thatclsxmust be added as a dependency.
This issue also appears on line 84 of the same file.
import {Card} from '@sanity/ui'
import {clsx} from 'clsx/lite'
import type {ComponentProps} from 'react'
import {floatingCard} from './FloatingCard.css'
function StyledFloatingCard({className, ...props}: ComponentProps<typeof Card>) {
return <Card {...props} className={clsx(floatingCard, className)} />
}
Summary
Adds a new agent skill,
.agents/skills/migrate-styled-components-to-vanilla-extract/SKILL.md— the step-by-step procedure for migrating one plugin's styling offstyled-componentsto vanilla-extract, distilled from:@sanity/google-maps-input(the reference implementation)sanity-plugin-workflow(clean mechanical migration on the current tsdown setup)@sanity/color-input(the dynamic-styling example:styleVariants,createVar+assignInlineVars,useTheme_v2(), migrating computed inlinestyleobjects)disableRuntimeStylesguidanceStack
Base branch:
cursor/ve-disable-runtime-styles-docs-9315(#1654), notmain— so the skill links the in-repoDisabling runtime styles in testssection of the styling reference directly instead of pointing at a PR. Merge after #1654 lands (or keep stacked).The skill covers the workflow (inventory table, three migration shapes,
tsdown.config.ts, dependency changes, Vitest wiring, snapshot update, verification, changeset, checklist) and defers to thesanity-plugin-best-practicesstyling reference for the underlying patterns, rather than duplicating them.Notable lessons encoded from the source PRs:
styled-componentsdevDependency: dropping it from a plugin that depends on@sanity/uican fork the plugin'ssanitypeer variant and break type-aware lint (Bugbot finding on Migrate sanity-plugin-workflow styling to vanilla-extract #1450). It can also work out fine (@sanity/color-inputdropped it after the@sanity/styled-componentsoverride removal in Remove @sanity/styled-components override, move back to styled-components #1693) — so the skill's rule is: remove the peer, then verify thesanity/@sanity/uiresolution strings inpnpm-lock.yamlstay aligned; keep thecatalog:devDep if they fork../bundle.cssexport block is written/synced automatically by the tsdown integration — nothing to hand-edit.style={{}}objects) migrate viacreateVar+assignInlineVars, with@vanilla-extract/dynamicas a runtimedependency(from refactor(color-input): migrate styling to vanilla-extract #1670).vanillaExtractPlugin()invitest.config.tsis required regardless ofdisableRuntimeStyles(which is an optional, per-plugin, jsdom-only opt-in — see the section docs: document vanilla-extract disableRuntimeStyles for Vitest #1654 adds).Cross-links
sanity-plugin-best-practices/SKILL.md+references/styling.mdnow point at the new skill for the procedure; the styling reference's post-migration dependency guidance ("Remove both once the migration is complete") is updated to match merged practice (remove the peer, verify lockfile peer-variant alignment before dropping the devDep).plugin-transfer/SKILL.md's "do not migrate styling during a transfer" notes now point follow-up work at the new skill.Testing
Docs-only change (no published packages touched, so no changeset).
pnpm format,pnpm lint, andpnpm knipall pass.