-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add some missing package dependencies #41486
Conversation
Build code transformations can introduce dependencies on packages such as @wordpress/element and @babel/runtime. These need to be declared if the package is to function correctly with yarn's p'n'p or pnpm with hoisting disabled. - @wordpress/preferences depends on @wordpress/element (fixes WordPress#41341) - @wordpress/reusable-blocks depends on @babel/runtime (fixes WordPress#41343) - @wordpress/viewport depends on @wordpress/element (fixes WordPress#41346)
384a281
to
93aea9b
Compare
Unit test failure in https://github.com/WordPress/gutenberg/actions/runs/5326375963/jobs/9648351381?pr=41486 seems unrelated. |
Thank you for adding the missing dependencies. Can you think of a programmatic way to prevent this type of issues to happen again?
Yes, that should be changed to a peer dependency when we move Node 16+ and the latest npm. |
Not offhand. You'd probably have to find/make some sort of tooling to extract all the deps from the built files and see that they're either depended on or peer-depended on by the package. |
Ok, it makes sense. We have some linting rules in place from eslint-plugin-import that work nicely with the source code, but they don't cover the parts dynamically modified by the Babel plugin that gets published to npm inside |
!-- Thanks for contributing to Gutenberg! Please follow the Gutenberg Contributing Guidelines:
https://github.com/WordPress/gutenberg/blob/trunk/CONTRIBUTING.md -->
What?
Add some missing package dependencies
Why?
Build code transformations can introduce dependencies on packages such
as @wordpress/element and @babel/runtime. These need to be declared if
the package is to function correctly with yarn's p'n'p or pnpm with
hoisting disabled.
@wordpress/preferences
is missing a dependency on@wordpress/element
#41341)@wordpress/reusable-blocks
is missing a dependency on@babel/runtime
#41343)@wordpress/viewport
is missing a dependency on@wordpress/element
#41346)How?
Adding the missing dependencies. Normally I'd probably have done the @babel/runtime dep as a peer dep, but I see you use a normal dependency everywhere else so I followed along.
Testing Instructions
yarn add <package>@file:/path/to/gutenberg/packages/<package>
or (with pnpm 7)pnpm add <package>@file:/path/to/gutenberg/packages/<package>
to point yarn or pnpm at the locally built version of the package.