Skip to content
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

Update @wordpress/components type definitions to 14.0.9 #60002

Merged

Conversation

sirbrillig
Copy link
Member

@sirbrillig sirbrillig commented Jan 12, 2022

Changes proposed in this Pull Request

This updates our dependency on the @wordpress/components type definitions (this is actually @types/wordpress__components since it uses DefinitelyTyped, not the package itself) to 14.0.9 which includes the types for ComboboxControl, ToolbarGroup, Guide, Tip and VisuallyHidden thanks to the following PRs:

Since we use all of the above components in calypso, this will fix our remaining TypeScript errors.

Note: the editing-toolkit app uses the Tip component, which works in TypeScript because that package adds its own type definition. Because this PR also adds a (more correct) type definition, it creates a type error in that package. It also creates an additional type error because that package is using Tip with a size prop that doesn't exist in the component's definition. Therefore, this PR removes both the override and the use of size.

Testing instructions

Verify that this doesn't change any behavior and affects only types.

@github-actions
Copy link

github-actions bot commented Jan 12, 2022

@sirbrillig sirbrillig self-assigned this Jan 12, 2022
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit update/wordpress-components-type-definitions-to-14-0-9 on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

@sirbrillig
Copy link
Member Author

sirbrillig commented Jan 12, 2022

The failing build reads:

  Yarn error YN0002: @types/wordpress__components@npm:14.0.9 doesn't provide react (p0930a), requested by downshift
  Yarn error YN0068: @types/wordpress__components ➤ peerDependencies ➤ react: No matching package in the dependency tree; you may not need this rule anymore.

...which is weird because none of the changes between @types/wordpress__components 14.0.5 and 14.0.9 should have touched CustomSelectControl and its dependency on downshift. 🤔

Also, those seem to be warnings and not errors so I'm not sure why they become errors at the bottom.

@tyxla
Copy link
Member

tyxla commented Jan 12, 2022

I'm assuming that WordPress/gutenberg#37578 fixed this, so if we upgrade the version of @wordpress/components we use in Calypso, the issue should get resolved. Haven't tested, though.

@sirbrillig
Copy link
Member Author

Interesting! Good find. However, it looks like we're already using the latest version, 19.2.0:

"@wordpress/components": "^19.2.0",
(and presumably that version includes the fix you linked to since the fix was merged 15 days ago and 19.2 was released 9 days ago).

@tyxla
Copy link
Member

tyxla commented Jan 12, 2022

Ah. Yep, so I guess we need another DT PR then 😉

@sirbrillig
Copy link
Member Author

sirbrillig commented Jan 12, 2022

🤔 I was about to do that, but then I looked and apparently no other DT types use peerDependencies, which seems concerning. I also found DefinitelyTyped/DefinitelyTyped#56011 and DefinitelyTyped/DefinitelyTyped#20290 which imply that they're not supported.

@tyxla
Copy link
Member

tyxla commented Jan 12, 2022

I see. In that case, we can specify the peer dependency as an exception in yarn's packageExtensions. It was already done FWIW, so I just updated the version in ff90d70. That should do the job.

@sirbrillig sirbrillig requested a review from a team January 12, 2022 21:34
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 12, 2022
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks! 🚢

@sirbrillig sirbrillig merged commit a3efc24 into trunk Jan 12, 2022
@sirbrillig sirbrillig deleted the update/wordpress-components-type-definitions-to-14-0-9 branch January 12, 2022 21:45
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants