-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: add Agent Skill for S1 to S2 migration #9778
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
Open
reidbarber
wants to merge
27
commits into
main
Choose a base branch
from
s1-to-s2-upgrade-skill
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+301
−32
Open
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
5b0ef13
fixed TabList render function children kept Item instead of convertin…
reidbarber be66290
import cleanup: removeComponentImport was only checking the first mat…
reidbarber 914f03c
dynamic import('@react-spectrum/s2') was incorrectly flagged as a v3 …
reidbarber d596a6c
Fixed in codemod options parsing
reidbarber 171529a
more import fixes
reidbarber a2509b5
add agent mode (non-interactive, run transforms only)
reidbarber 48f1768
add e2e tests
reidbarber ff6271f
have --components include related components
reidbarber 5c023f9
yarn.lock
reidbarber b871a88
update outdated links
reidbarber 6186108
Merge branch 'main' into codemod-fixes
reidbarber 4d6af4c
bump @adobe/react-spectrum version
reidbarber 323ad01
fix missing imports and remove unused
reidbarber 9bb8185
Merge remote-tracking branch 'origin/main' into codemod-fixes
reidbarber 3e71ffb
fix snapshot
reidbarber 83b52c6
yarn.lock
reidbarber 62a60ab
map illustrations to s2 illustrations
reidbarber d4dfa4e
Merge remote-tracking branch 'origin/main' into codemod-fixes
reidbarber 631e45a
initialize migration skill
reidbarber 693d822
typo
reidbarber 202903e
update README
reidbarber eb12fc2
Merge branch 'main' into s1-to-s2-upgrade-skill
reidbarber 8a70ab7
Merge remote-tracking branch 'origin/main' into s1-to-s2-upgrade-skill
reidbarber eacd571
Merge branch 's1-to-s2-upgrade-skill' of https://github.com/adobe/rea…
reidbarber 80d9703
add more content to skill content
reidbarber f39e420
Merge remote-tracking branch 'origin/main' into s1-to-s2-upgrade-skill
reidbarber a6bd663
lint
reidbarber File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
packages/dev/s2-docs/migration-references/focused-app-setup.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # App setup and style macro | ||
|
|
||
| ## Full-page apps | ||
|
|
||
| - Import `@react-spectrum/s2/page.css` in the app entrypoint so the page background and color scheme are applied before JavaScript runs. | ||
| - Do not carry over a v3 root `Provider theme={defaultTheme}` wrapper just to make the app render. S2 does not require that pattern. | ||
| - Do not assume there is only one app root. Standalone pages, alternate entrypoints, utility apps, embedded sub-apps, and test-only render targets may each need their own `page.css` or Provider cleanup. | ||
|
|
||
| ## Embedded sections | ||
|
|
||
| - If S2 renders as part of a larger page instead of owning the whole document, keep the S2 subtree inside `<Provider background="...">` rather than importing `page.css` globally. | ||
|
|
||
| ## Provider decisions | ||
|
|
||
| - Use S2 `Provider` when you need locale overrides, router integration, explicit color-scheme/background control, or SSR with `elementType="html"`. | ||
| - Remove `theme={defaultTheme}` and other v3 theme props. They do not carry forward to S2. | ||
| - Preserve surrounding app-shell providers such as routing, Redux/store, analytics, i18n/intl, and product-framework host providers. Replace only the React Spectrum-specific wrapper instead of flattening the whole provider stack. | ||
| - For tests, wrap only the cases that actually need S2 context such as locale or background. Do not recreate a full v3 theme wrapper by default. | ||
|
|
||
| ## Bundlers | ||
|
|
||
| - Detect the bundler at the migration target level, especially in monorepos. The workspace root may include Storybook, Vite, or other tooling that does not represent the runtime bundler for the package being migrated. | ||
| - Parcel v2.12.0+ already supports S2 macros. Most Parcel repos only need the package install and the right entrypoint CSS import. | ||
| - Non-Parcel bundlers need `unplugin-parcel-macros` and the appropriate framework setup. Keep plugin ordering correct so macros run before the rest of the toolchain. | ||
| - If the repo already has a framework-specific S2 setup, preserve it instead of layering a second macro configuration on top. |
37 changes: 37 additions & 0 deletions
37
packages/dev/s2-docs/migration-references/focused-manual-fixes.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Manual fixes after the codemod | ||
|
|
||
| ## Known gaps to handle explicitly | ||
|
|
||
| - `@react-spectrum/toast` imports are not automatically migrated. Move them to S2, then re-check `ToastContainer` mounts and `ToastQueue` usages. | ||
| - v3 `Provider` and `defaultTheme` wrappers in apps and tests need human review. S2 does not use the v3 theme object model. | ||
|
|
||
| ## Imports and packages | ||
|
|
||
| - Collapse v3 component imports onto `@react-spectrum/s2`. | ||
| - Keep using `@spectrum-icons/*` only when there is no S2 icon or illustration equivalent. Prefer `@react-spectrum/s2/icons/*` and `@react-spectrum/s2/illustrations` when possible. | ||
| - If the codemod leaves `TODO(S2-upgrade)` next to an icon or illustration import, pick the nearest S2 replacement manually. | ||
|
|
||
| ## App and Provider setup | ||
|
|
||
| - Full-page apps usually add `import '@react-spectrum/s2/page.css';` at the entrypoint and no longer need a mandatory root Provider just to supply `theme={defaultTheme}`. | ||
| - Embedded sections still use S2 `Provider` with an explicit `background`. | ||
| - Keep or add S2 `Provider` only when locale, router integration, color-scheme/background overrides, or SSR `elementType="html"` behavior are needed. | ||
| - Preserve unrelated wrappers such as routing, store, analytics, i18n, and host-framework providers. Replace or remove only the React Spectrum-specific layer. | ||
|
|
||
| ## Style and layout follow-ups | ||
|
|
||
| - Convert v3 style props and `UNSAFE_style` cases to the S2 style macro when possible. | ||
| - `Flex` and `Grid` often become `div` elements styled with the macro. | ||
| - Review `ClearSlots` and other direct `@react-spectrum/utils` imports manually. These are not part of the common S2 app surface. | ||
|
|
||
| ## Dialogs and collections | ||
|
|
||
| - `DialogContainer` and `useDialogContainer` still exist in S2, but the import path changes and dismiss logic may need to move between `Dialog`, `DialogTrigger`, and `DialogContainer`. | ||
| - When `Item` survives the codemod, rename it based on its parent: `MenuItem`, `PickerItem`, `ComboBoxItem`, `ListBoxItem`, `Tab`, `TabPanel`, `Tag`, `Breadcrumb`, and similar. | ||
| - Preserve React `key` when mapping arrays, but ensure collection data items expose `id` when S2 expects it. | ||
| - Table and ListView migrations often need manual review for row headers, nested columns, and explicit item ids. | ||
|
|
||
| ## Tests | ||
|
|
||
| - Replace v3 Provider/defaultTheme test wrappers with the minimal S2 `Provider` props the test actually needs, or remove the wrapper entirely if no S2 context is required. | ||
| - Update toast mocks and assertions that still reference `@react-spectrum/toast` or old dialog markup. | ||
22 changes: 22 additions & 0 deletions
22
packages/dev/s2-docs/migration-references/focused-toast.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Toast migration | ||
|
|
||
| ## Import changes | ||
|
|
||
| - Move `ToastContainer` and `ToastQueue` imports from `@react-spectrum/toast` to `@react-spectrum/s2`. | ||
| - Keep a shared `ToastContainer` mounted near the app root or test harness, then update all queue calls to use the S2 import path. | ||
|
|
||
| ## Queue methods | ||
|
|
||
| - S2 supports `ToastQueue.neutral`, `positive`, `negative`, and `info`. | ||
| - Re-check options such as `timeout`, `actionLabel`, `onAction`, `shouldCloseOnAction`, and `onClose` after the import move. | ||
| - The queue methods still return a close function. Keep programmatic dismissal logic when the existing UX depends on it. | ||
|
|
||
| ## Common post-codemod blind spots | ||
|
|
||
| - Search for every `ToastContainer` mount and every `ToastQueue` usage after moving imports. Shared app roots, secondary entrypoints, and test harnesses are easy to miss. | ||
|
|
||
| ## Tests and mocks | ||
|
|
||
| - Update every toast mock to point at `@react-spectrum/s2`. | ||
| - If a test mounted `ToastContainer` from the old package, swap it to the S2 import as part of the same change. | ||
| - Re-run the affected tests after the import move. Toast helpers are often mocked in many files and are easy to miss. |
45 changes: 45 additions & 0 deletions
45
packages/dev/s2-docs/migration-references/focused-workflow.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # Codemod-first workflow | ||
|
|
||
| ## Inspect before editing | ||
|
|
||
| - Search package manifests and source for `@adobe/react-spectrum`, `@react-spectrum/*`, and `@spectrum-icons/*`. | ||
| - In monorepos or mixed-tooling repos, inspect the target package or app first instead of assuming the root manifest, Storybook config, or workspace tooling represents the runtime target being migrated. | ||
| - Determine the package manager from the relevant lockfile or workspace setup, then choose the codemod runner that matches that repo or package. | ||
| - Detect the bundler at the migration target level before touching setup files. Parcel v2.12.0+ already supports S2 style macros. Vite, webpack, Next.js, Rollup, ESBuild, and similar toolchains need explicit macro-plugin setup. | ||
| - Find app entrypoints, standalone pages, alternate entrypoints, embedded sub-apps, utility apps, test-only render targets, root providers, shared test wrappers, toast setup, and any direct `defaultTheme` usage before the codemod changes imports. | ||
| - Search for `ToastContainer`, `ToastQueue`, `DialogContainer`, `useDialogContainer`, `ClearSlots`, style props, and `UNSAFE_style`. These are common follow-up areas after the codemod. | ||
|
|
||
| ## Run the codemod first | ||
|
|
||
| Prefer the repo-native non-interactive command so the upgrade stays deterministic: | ||
|
|
||
| ```bash | ||
| npx @react-spectrum/codemods s1-to-s2 --agent | ||
| yarn dlx @react-spectrum/codemods s1-to-s2 --agent | ||
| pnpm dlx @react-spectrum/codemods s1-to-s2 --agent | ||
| ``` | ||
|
|
||
| Use `npx` for npm and Yarn 1 repos, `yarn dlx` for Yarn Berry or Yarn PnP repos, and `pnpm dlx` for pnpm repos. Use the equivalent workspace-native runner if the repo uses another package manager. | ||
| Use `--path <dir>` for monorepos or when only one package should migrate first. In a monorepo, run the codemod against the target subtree first instead of the whole workspace. | ||
| Use `--components A,B` when the user explicitly wants an incremental rollout by component family. | ||
| Use `--dry` when you need to preview scope before editing. | ||
|
|
||
| ## Resolve follow-up work in order | ||
|
|
||
| 1. Install or verify `@react-spectrum/s2` and clean up imports. | ||
| 2. Add S2 app setup such as `@react-spectrum/s2/page.css` or `Provider` changes. | ||
| 3. Search for `TODO(S2-upgrade)` and fix every remaining comment. | ||
| 4. Resolve style prop, layout, and dialog/collection follow-ups. | ||
| 5. Migrate icons, illustrations, and toast imports. | ||
| 6. Update tests, mocks, and validation commands. | ||
|
|
||
| ## Validate with repo-native commands | ||
|
|
||
| Prefer the narrowest existing scripts from `package.json`: | ||
|
|
||
| - dependency install if manifests changed | ||
| - typecheck or compile | ||
| - focused tests for touched areas | ||
| - build | ||
|
|
||
| For monorepos, validate the affected package or subtree first with its own scripts before escalating to workspace-wide checks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
i thought we changed it so that the S2 Provider was always needed in order to load the font? originally it was optional but i thought it sorta became mandatory