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

Add JavaScript Documentation around setting up Webpack, Babel, and Building #13629

Merged
merged 10 commits into from
Feb 7, 2019

Conversation

mkaz
Copy link
Member

@mkaz mkaz commented Feb 1, 2019

Description

Adds a new page to the JavaScript tutorial explaining how to setup webpack, babel, and building ESNext and JSX.

This updates webpack and babel versions which differ from gutenberg-examples repo, so if we are good with the updates I can create some PRs to apply the same changes there.

How has this been tested?

Confirm the code works as explained.
View page on branch

Types of changes

Documentation.

I updated the table of contents, moving the JavaScript tutorial ahead of the Block Tutorial, since that is a more accurate order of knowledge needed. This also better for the way navigation works in the Handbook.

@mkaz mkaz self-assigned this Feb 1, 2019
@mkaz mkaz added [Type] Developer Documentation Documentation for developers [Status] In Progress Tracking issues with work in progress labels Feb 1, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

@mkaz thank you for starting this PR. I will have a closer look next week. I left some comments with some details about the recent discussions around all the tooling related to this tutorial.

```json
"scripts": {
"dev": "webpack --watch",
"build": "cross-env NODE_ENV=production webpack"
Copy link
Member

Choose a reason for hiding this comment

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

See how it is handled in Gutenberg with build script from wp-scripts exposed by @wordpress/scripts package:

https://github.com/WordPress/gutenberg/blob/master/package.json#L152

Copy link
Contributor

@chrisvanpatten chrisvanpatten left a comment

Choose a reason for hiding this comment

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

This is a great start. I've added a mix of grammar fixes and formatting tweaks. Excited to see this move forward!

@chrisvanpatten chrisvanpatten dismissed their stale review February 1, 2019 18:08

Thanks @mkaz for tackling those fixes. Dismissing my review so I don't block merge if someone else approves 👍

@mkaz
Copy link
Member Author

mkaz commented Feb 1, 2019

@gziolo @youknowriad I can use the wp-scripts package and it does help simplify some pieces, but it also includes a lot of extra dependencies, such as Chromium, jest, and other parts that aren't necessary when just starting to learn about webpack/babel

Here's the install summary for scripts:

+ @wordpress/scripts@2.5.0
added 1184 packages from 538 contributors and audited 23175 packages in 37.032s
found 0 vulnerabilities

@youknowriad
Copy link
Contributor

youknowriad commented Feb 1, 2019

@mkaz Good point about the dependencies, this is probably something that needs improvement separately as not everyone uses e2e tests and don't need Chrome... A solution could be to install puppeteer the first time you launch wp-scripts test-e2e. The other dependencies are less concerning for me.

In the end, the user only has to add one item (at least very few items) into its package.json and the setup is simple.

Overall, what's important I think is that we should be consistent. We're building the scripts package for the same reason this README is being written. Help people use a consistent and modern setup.

@chrisvanpatten
Copy link
Contributor

I tend to agree w/ @youknowriad on that matter. It's not ideal that the script package installs so much out of the gate, but it still feels like overall a better user experience.

@gziolo
Copy link
Member

gziolo commented Feb 4, 2019

Thanks @mkaz for tackling those fixes. Dismissing my review so I don't block merge if someone else approves 👍

Aside: in case you didn't know, you can batch suggestion commits when you go to Files changed tab :)

I can use the wp-scripts package and it does help simplify some pieces, but it also includes a lot of extra dependencies, such as Chromium, jest, and other parts that aren't necessary when just starting to learn about webpack/babel

Yes, this is unfortunate, but it is what it is. You can make the aforementioned puppeteer a peerDependency but then someone will have to install it themselves to use the command. I personally opted for the simplest approach assuming that you npm i only once and forget when using your plugin or theme. We can revisit it later by playing with npx (last time I tried it installs chromium every time you run it...) or something similar.

@gziolo gziolo requested a review from mcsf February 5, 2019 15:59
@gziolo gziolo added this to the Documentation & Handbook milestone Feb 5, 2019
@mkaz mkaz removed the [Status] In Progress Tracking issues with work in progress label Feb 7, 2019
Copy link
Contributor

@chrisvanpatten chrisvanpatten left a comment

Choose a reason for hiding this comment

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

I just have some grammar changes; otherwise this seems ready to me unless anyone else notices anything :)

Co-Authored-By: mkaz <marcus@mkaz.com>
Co-Authored-By: mkaz <marcus@mkaz.com>
@mkaz mkaz merged commit c9357bf into master Feb 7, 2019
@mkaz mkaz deleted the docs/js2 branch February 7, 2019 19:43
@youknowriad
Copy link
Contributor

Would be good to follow-up on this and focus on the scripts abstraction instead of the low-level tools.

@gziolo
Copy link
Member

gziolo commented Feb 15, 2019

There is #13712 opened to address it. I have it on my todo list 👍

youknowriad pushed a commit that referenced this pull request Mar 6, 2019
…ilding (#13629)

* Add JS Build Setup

* Update docs/designers-developers/developers/tutorials/javascript/js-build-setup.md
Co-Authored-By: mkaz <marcus@mkaz.com>

* Whitespace and code brackets

* Update with better npm install instructions using --save-dev

* Update docs/designers-developers/developers/tutorials/javascript/versions-and-building.md

Co-Authored-By: mkaz <marcus@mkaz.com>

* Update docs/designers-developers/developers/tutorials/javascript/js-build-setup.md

Co-Authored-By: mkaz <marcus@mkaz.com>

* Updates node, cli, and pragma per review

* Update why to use ESNext

* Apply suggestions from code review

Co-Authored-By: mkaz <marcus@mkaz.com>

* Apply suggestions from code review

Co-Authored-By: mkaz <marcus@mkaz.com>
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
…ilding (#13629)

* Add JS Build Setup

* Update docs/designers-developers/developers/tutorials/javascript/js-build-setup.md
Co-Authored-By: mkaz <marcus@mkaz.com>

* Whitespace and code brackets

* Update with better npm install instructions using --save-dev

* Update docs/designers-developers/developers/tutorials/javascript/versions-and-building.md

Co-Authored-By: mkaz <marcus@mkaz.com>

* Update docs/designers-developers/developers/tutorials/javascript/js-build-setup.md

Co-Authored-By: mkaz <marcus@mkaz.com>

* Updates node, cli, and pragma per review

* Update why to use ESNext

* Apply suggestions from code review

Co-Authored-By: mkaz <marcus@mkaz.com>

* Apply suggestions from code review

Co-Authored-By: mkaz <marcus@mkaz.com>
This was referenced Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants