tsgo: Fix js-packages/connection type errors#47380
tsgo: Fix js-packages/connection type errors#47380manzoorwanijk wants to merge 1 commit intotrunkfrom
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
This PR addresses TypeScript (tsgo) type errors in the @automattic/jetpack-connection package by converting the useConnection hook to TypeScript and tightening up typing around connection store selectors and error handling.
Changes:
- Convert
components/use-connectionfrom JS to TS, introducing explicit prop/return interfaces and additional data types. - Update selector usage in
useConnectionto add explicit typing/casts and avoid spreading opaque selector return values. - Fix a union-narrowing issue in the connect screen (
registrationErrorcan befalse) and usewindow.location.hreffor type-safe redirects.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/js-packages/connection/package.json | Updates the subpath export for ./use-connection to point at the new TS entry file. |
| projects/js-packages/connection/components/use-connection/types.ts | Adds exported TS interfaces for hook props/return types and related data shapes. |
| projects/js-packages/connection/components/use-connection/index.ts | Converts the hook implementation to TS and applies typed useSelect/selector reads and redirect typing. |
| projects/js-packages/connection/components/connect-screen/basic/index.tsx | Adjusts registrationError access to properly narrow the false union branch. |
| projects/js-packages/connection/changelog/update-tsgo-fix-use-connection-type-errors | Adds a patch changelog entry for the TS conversion. |
Comments suppressed due to low confidence (1)
projects/js-packages/connection/components/use-connection/index.ts:60
- The hook signature claims to return
userConnectionData: UserConnectionData, but heregetUserConnectionData()is only type-cast and not runtime-normalized. Since the selector can returnfalse, this can leakfalseto callers who will treat it as an object based on the TS types. Consider coercing falsy values to a safe default (or updating the hook return type to includefalse).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/js-packages/connection/components/use-connection/types.ts
Outdated
Show resolved
Hide resolved
projects/js-packages/connection/components/use-connection/types.ts
Outdated
Show resolved
Hide resolved
projects/js-packages/connection/components/use-connection/types.ts
Outdated
Show resolved
Hide resolved
Convert useConnection hook from JS to TypeScript with explicit
UseConnectionProps and UseConnectionReturn interfaces, fixing 5
TS2339 errors caused by tsgo inferring the return type as '{}'.
0fa3f46 to
adce030
Compare
Code Coverage Summary2 files are newly checked for coverage.
|
Fixes MONOREP-377
Proposed changes:
Fix tsgo type errors in
js-packages/connectionby converting theuseConnectionhook from JavaScript to TypeScript.TS2339 — Properties don't exist on type
'{}':Converted
components/use-connection/index.jsxto.tswithUseConnectionPropsandUseConnectionReturninterfaces, giving the hook explicit parameter and return types. Extracted types into a separatetypes.tsfile. Added concreteUserConnectionData,WpcomUser, andRegistrationErrorinterfaces so downstream consumers (e.g.my-jetpack) get proper property access throughReturnType<typeof useConnection>.TS2339 —
responsedoes not exist on typefalse:Changed
registrationError?.response?.codeto a truthiness-narrowed ternary inconnect-screen/basic/index.tsx, since optional chaining doesn't narrow thefalsebranch of the union type.Untyped
useSelectcallbacks:Added explicit
StoreSelectortype for theselectparameter and cast store selector return values to their expected types, replacing the opaquegetConnectionStatus()spread with individually typed properties.window.location = redirectUri:Changed to
window.location.href = redirectUrifor type-safe assignment.Other information:
Jetpack product discussion
N/A — Internal tooling change.
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
pnpm typecheckand verify cascading errors are also resolved.Changelog
Note: This is not the type of TypeScript I like but it's more like a band-aid for the problem. The real fix would be to convert the whole connection data store to TS and use the store object in useSelect and useDispatch instead of the store ID.