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

Upgrade @wordpress/scripts and add ESLint support for ESNext code #83

Merged
merged 2 commits into from
Aug 7, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Aug 5, 2019

Closes #49.

This PR adds global support for @wordpress/scripts in the project. This allows simplifying the setup of individual blocks which had to use it in the past making the whole project setup much simpler. Instead of having to run npm install in 4 packages which use ESNext, we can now do it once in the root folder and run npm run build to rule them all.

In addition, this PR adds ESLint support for ESNext based packages. We could enable it for other blocks which are ES5 based but we would need to upgrade ESLint to v6 which supports overrides fro extends feature from the config file. I will figure out how to do it in @wordpress/scripts and update here once published to npm.

I also opened a follow-up PR WordPress/gutenberg#16904 so you don't have to silence ESLint error when using wp global.

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

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

Looks good, built and tested each of the blocks. Thanks for updating!
Glad to see the input/output parameters now available for wp scripts

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

Per this weeks #core-js chat this week can we follow up the changes discussed please.

Primarily the changes requested are to not remove package.json files from each of the examples, this is so that developers can copy and paste an entire example folder to their and do not have to worry about trying to cheery pick npm commands from the root of the project.

@gziolo
Copy link
Member Author

gziolo commented Aug 7, 2019

Primarily the changes requested are to not remove package.json files from each of the examples, this is so that developers can copy and paste an entire example folder to their and do not have to worry about trying to cheery pick npm commands from the root of the project.

That is indeed a good point. I addressed it in 85274c9. I ensured that all commands managed globally can also be executed when the individual block is copied over.

I thought about adding lerna so we could run those commands from the root folder in a more simplified way, but this would prevent us from having examples of more advanced usage of wp-scripts. So, I guess it's fine as is until we have double as much ESNext bases subpackages to maintain.

@gziolo gziolo mentioned this pull request Aug 7, 2019
Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

Thanks @gziolo 💯

@ntwb ntwb merged commit 34e7d7c into master Aug 7, 2019
@ntwb ntwb deleted the update/wordpress-scripts branch August 7, 2019 08:31
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include sample ESLint configuration
3 participants