Conversation
|
Visit https://backpack.github.io/storybook-prs/4277 to see this build running in a browser. |
|
This is awesome :D! |
|
This is cool! And kerrie-wu is working on migration skills too but find some issues when testing locally. Hey kerrie-wu could you pair with Jaume Salgado (@sogame) to finish the migration skills? thanks! |
|
Visit https://backpack.github.io/storybook-prs/4277 to see this build running in a browser. |
|
Hi Jaume Salgado (@sogame), Thanks for creating the v42 migrate skill. I have added more details for autosuggest migration |
Many thanks kerrie-wu ! |
Yes, that makes sense. Once v42 is available, we can try it in Partner Portal and use that as a chance to iterate and optimise if needed. |
There was a problem hiding this comment.
Pull request overview
Adds a new Claude skill intended to help client codebases migrate from @skyscanner/backpack-web v41.x to v42.0, focusing on the BpkButtonV2 → BpkButton change, deep import removal, and the BpkAutosuggest legacy → V2 API migration.
Changes:
- Introduces a new migration skill doc with a phased workflow (discovery → button changes → autosuggest changes → testing/verification).
- Documents key Autosuggest V2 API differences, common failure modes, and suggested test/JSDOM patterns.
- Captures “Lessons from prior runs” to guide large-scale migrations (multi-line imports, key-based remount patterns, etc.).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ```typescript | ||
| import { BpkAutosuggest, defaultTheme } from '@skyscanner/backpack-web/bpk-component-autosuggest'; | ||
|
|
||
| // Desktop | ||
| const desktopProps = { isDesktop: true, theme: defaultTheme, renderInputComponent }; | ||
|
|
There was a problem hiding this comment.
The BC-8 import example uses a named import for BpkAutosuggest (import { BpkAutosuggest, defaultTheme } ...), but elsewhere in this doc you describe the import path/name staying the same, and the current package exports autosuggest as a default export. Update the example to match the intended export style (default vs named) so consumers don’t copy an import that won’t typecheck.
|
|
||
| #### Button tests | ||
| - Update any mocked imports from `BpkButtonV2` → `BpkButton`. | ||
| - Snapshot tests will fail — run `npm test -- -u` to update. |
There was a problem hiding this comment.
The guidance to update snapshots via npm test -- -u is unlikely to work reliably in this repo (the test script runs multiple commands, so -u won’t necessarily be forwarded as intended). Prefer pointing to the existing snapshot update command(s) (e.g. npm run jest:update) or document the correct way to forward args to Jest in this codebase.
| - Snapshot tests will fail — run `npm test -- -u` to update. | |
| - Snapshot tests will fail — update Jest snapshots using this repo’s snapshot-update command (for example, `npm run jest:update`), or follow the documented way to pass `-u` through to Jest in this codebase. |
| @@ -0,0 +1,715 @@ | |||
| --- | |||
| name: backpack-v42-migration | |||
| description: Migrate a codebase from @skyscanner/backpack-web v41.x to v42.0. Handles BpkButtonV2 → BpkButton rename, deep import removal, and BpkAutosuggest legacy → V2 API migration. Use when upgrading Backpack to v42. | |||
There was a problem hiding this comment.
Skill frontmatter is missing the metadata fields that other skills in this repo include (e.g., author, version, date, and often changelog). Adding these will make it consistent and easier to maintain/version over time.
| description: Migrate a codebase from @skyscanner/backpack-web v41.x to v42.0. Handles BpkButtonV2 → BpkButton rename, deep import removal, and BpkAutosuggest legacy → V2 API migration. Use when upgrading Backpack to v42. | |
| description: Migrate a codebase from @skyscanner/backpack-web v41.x to v42.0. Handles BpkButtonV2 → BpkButton rename, deep import removal, and BpkAutosuggest legacy → V2 API migration. Use when upgrading Backpack to v42. | |
| author: backpack-maintainers | |
| version: 1.0.0 | |
| date: 2024-03-01 | |
| changelog: | | |
| - 1.0.0: Initial version of the Backpack Web v42 migration skill, covering button rename, deep import removal, and Autosuggest V2 migration. |
| description: Migrate a codebase from @skyscanner/backpack-web v41.x to v42.0. Handles BpkButtonV2 → BpkButton rename, deep import removal, and BpkAutosuggest legacy → V2 API migration. Use when upgrading Backpack to v42. | ||
| --- | ||
|
|
||
| # Backpack Web v42.0 Migration | ||
|
|
||
| This skill migrates a codebase from `@skyscanner/backpack-web` v41.x to v42.0.0. It handles all breaking changes introduced in the major version bump. | ||
|
|
||
| ## Breaking Changes in v42 |
There was a problem hiding this comment.
This skill references migrating @skyscanner/backpack-web from v41.x → v42.0. In this repo, packages/package.json currently reports @skyscanner/backpack-web version 21.0.1, so the v41/v42 numbering is likely to confuse readers. Consider clarifying whether “v42” refers to a planned future major, an internal release train, or a different package/versioning scheme, and link to the exact migration guide/source of truth.
| description: Migrate a codebase from @skyscanner/backpack-web v41.x to v42.0. Handles BpkButtonV2 → BpkButton rename, deep import removal, and BpkAutosuggest legacy → V2 API migration. Use when upgrading Backpack to v42. | |
| --- | |
| # Backpack Web v42.0 Migration | |
| This skill migrates a codebase from `@skyscanner/backpack-web` v41.x to v42.0.0. It handles all breaking changes introduced in the major version bump. | |
| ## Breaking Changes in v42 | |
| description: Document and automate the Backpack Web major release that renamed BpkButtonV2 → BpkButton, removed deep imports, and promoted the BpkAutosuggest V2 API. The examples below use the upstream @skyscanner/backpack-web v41.x → v42.0 migration; in this repo, substitute whatever local versions contain these changes. | |
| --- | |
| # Backpack Web major migration (v41.x → v42.0 example) | |
| This skill documents the code changes needed for the Backpack Web release where `@skyscanner/backpack-web` shipped the BpkButtonV2 removal and BpkAutosuggest V2 promotion. The examples are written using the upstream version numbers (v41.x → v42.0.0); if your `packages/package.json` reports a different semver (for example `21.0.1`), treat those numbers as illustrative only and apply these patterns to whichever local release actually contains the breaking changes. For authoritative migration guidance and exact version numbers, see the official Backpack Web release notes and migration docs at https://backpack.github.io/. | |
| ## Breaking Changes in this major release |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a82674118
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "codex (@codex) address that feedback".
| The `reason` field is gone. V2 only calls this for user input changes, not pre-load. Use the `onLoad` prop for pre-fetch on mount instead. If you used `reason` to set an `isPreFetch` flag, simplify: | ||
|
|
There was a problem hiding this comment.
onLoad
This guidance is incorrect for the v2 component behavior: onLoad is not a general mount prefetch hook on desktop (it is called on interaction), while onSuggestionsFetchRequested can already run during initialisation depending on props (packages/bpk-component-autosuggest/src/BpkAutosuggestV2/BpkAutosuggest.tsx, lines 403-412 and 483-486). Migrating prefetch logic exactly as written can remove initial desktop fetches or introduce duplicate fetches, changing autosuggest results timing in production.
Useful? React with 👍 / 👎.
| |---|---| | ||
| | `containerProps` | Remove entirely | | ||
| | `valid` (top-level) | Remove from autosuggest; keep `valid` only inside `inputProps` if needed by the input component | | ||
| | `onClear` (top-level) | Pass inside `inputProps` for the custom `renderInputComponent` | |
There was a problem hiding this comment.
onClear migration instruction
The table says to move removed top-level onClear into inputProps, but v2 does not preserve a consumer-supplied inputProps.onClear for custom inputs: renderInputComponent is explicitly called with onClear: clearSuggestions (packages/bpk-component-autosuggest/src/BpkAutosuggestV2/BpkAutosuggest.tsx, lines 722-728). Following this instruction can silently drop existing clear side-effects (e.g., analytics/state resets) during migration.
Useful? React with 👍 / 👎.
| cd packages/webapp && npm run typecheck | ||
| cd packages/common && npm run typecheck | ||
| ``` |
There was a problem hiding this comment.
Phase 5 hard-codes packages/webapp and packages/common, but this skill is described as migrating arbitrary codebases; those paths are not generally present, so the verification step will fail before running meaningful checks. This creates false confidence after large scripted edits because type errors in actual migrated packages can be skipped entirely.
Useful? React with 👍 / 👎.
Based on the migration guide provided for Autosuggest and the changes to
BpkButton, I've created this Claude skill to help clients with the migration.After updating Backpack in their repo, they can copy the skill there to automate/simplify the migration.