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

Block Editor: Add missing dependencies to package #16628

Closed
wants to merge 2 commits into from
Closed

Block Editor: Add missing dependencies to package #16628

wants to merge 2 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Jul 17, 2019

Description

I'm using @wordpress/block-editor as a dependency for a Jetpack PR, Automattic/jetpack#13070.

During build, webpack complained about a number of missing dependencies, which seems warranted:

This PR is adding them to block-editor's package.json

How has this been tested?

(Doesn't really need testing since I'm just explicitly adding dependencies that were previously used implicitly.)

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@swissspidy
Copy link
Member

Travis complains:

> gutenberg@6.1.1 check-local-changes /home/travis/build/WordPress/gutenberg
> ( git diff -U0 | xargs -0 node bin/process-git-diff ) || ( echo "There are local uncommitted changes after one or both of 'npm install' or 'npm run docs:build'!" && exit 1 );
There are local uncommitted changes after one or both of 'npm install' or 'npm run docs:build'!

@ockham
Copy link
Contributor Author

ockham commented Jul 18, 2019

@swissspidy Any idea what I'm doing wrong? I've run both npm i and 'npm run docs:build locally but don't really see any changes. git diff -U0 | xargs -0 node bin/process-git-diff also comes back empty... 🤔

@@ -28,6 +28,7 @@
"@wordpress/blocks": "file:../blocks",
"@wordpress/components": "file:../components",
"@wordpress/compose": "file:../compose",
"@wordpress/core-data": "^2.4.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

block editor shouldn't depend on core-data we should instead refactor it to remove the need for this dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgefilipecosta do you know why we still need this?

Copy link
Member

Choose a reason for hiding this comment

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

Could we get at least react-autosize-textarea and traverse bits in independently, then?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that block-editor should not depend on core-data, and I don't know the reason we have this dependency. Maybe we should just remove the import from: https://github.com/WordPress/gutenberg//blob/a3ef9605c74a8f3e6cb57a28fad33e4701585d6a/packages/block-editor/src/index.js#L5

In my tests, it seems to work fine.

Copy link
Member

Choose a reason for hiding this comment

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

@jorgefilipecosta can you help to remove it in another PR? :-)

Copy link
Member

Choose a reason for hiding this comment

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

Hi @simison, I'm sorry for the delay I had some AFK time. I added this issue to my to-do list.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like @gziolo solved this problem in PR #16969.

Copy link
Member

Choose a reason for hiding this comment

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

This and way much more of issues :)

@gziolo
Copy link
Member

gziolo commented Aug 9, 2019

Fixed in #16969. @ockham, thanks for opening this PR. I didn't notice it earlier, sorry about it.

@gziolo gziolo closed this Aug 9, 2019
@ockham ockham deleted the add/block-editor/missing-dependencies branch August 14, 2019 14:28
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.

6 participants