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 wp-env docs to reflect current functionality of package #21809

Merged
merged 10 commits into from
Apr 24, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Apr 23, 2020

Fixes #20923

Description

@ajitbohra mentioned on slack that some of the instructions in the wp-env README either didn't work or weren't clear enough.

This PR updates those docs:

  • Quick start instructions didn't previously work as this depends on wp-env being published on NPM as the wp-env package, but it's published instead as @wordpress/env.
  • Instructions centered around installing wp-env globally. This update makes it clear that wp-env can also be used locally (as it's used in Gutenberg).
  • The README now also mentions that wp-env is intended to be used with the latest LTS version of Node.

Types of changes

Documentation

@talldan talldan added [Type] Developer Documentation Documentation for developers [Tool] Env /packages/env labels Apr 23, 2020
@talldan talldan requested a review from noahtallen as a code owner April 23, 2020 06:43
@talldan talldan self-assigned this Apr 23, 2020
@github-actions
Copy link

github-actions bot commented Apr 23, 2020

Size Change: +94 B (0%)

Total Size: 845 kB

Filename Size Change
build/block-editor/index.js 105 kB +17 B (0%)
build/block-library/index.js 112 kB +30 B (0%)
build/data-controls/index.js 1.29 kB +40 B (3%)
build/editor/index.js 43.4 kB +7 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/style-rtl.css 10.1 kB 0 B
build/block-editor/style.css 10.1 kB 0 B
build/block-library/editor-rtl.css 7.13 kB 0 B
build/block-library/editor.css 7.13 kB 0 B
build/block-library/style-rtl.css 7.19 kB 0 B
build/block-library/style.css 7.19 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 28.2 kB 0 B
build/edit-post/style-rtl.css 12.5 kB 0 B
build/edit-post/style.css 12.5 kB 0 B
build/edit-site/index.js 10.8 kB 0 B
build/edit-site/style-rtl.css 5.25 kB 0 B
build/edit-site/style.css 5.24 kB 0 B
build/edit-widgets/index.js 8.33 kB 0 B
build/edit-widgets/style-rtl.css 5 kB 0 B
build/edit-widgets/style.css 5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/style-rtl.css 3.27 kB 0 B
build/editor/style.css 3.27 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.32 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action


```json
"scripts": {
"wp-env": "packages/env/bin/wp-env"
Copy link
Member

@ajitbohra ajitbohra Apr 23, 2020

Choose a reason for hiding this comment

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

seems like we don't require this npm run wp-env start without this works. This path is in the context of Gutenberg will not work externally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about the path. I think this may be needed for more advanced usage to be able to add extra arguments to a command e.g.

npm run wp-env run composer run-script lint

but I haven't tested that. Possibly the path would have to change to the node_modules folder, or maybe it does already work without further changes.

Copy link
Member

@ajitbohra ajitbohra Apr 23, 2020

Choose a reason for hiding this comment

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

 "scripts": {
    "env:start": "wp-env start",
    "env:stop": "wp-env stop",
    "php:lint": "wp-env run composer run-script lint"
  }

or

"scripts": {
    "env": "wp-env",
  }
npm run env start
npm run env stop
npm run env run composer run-script lint

this works fine without specifying the path to command.

the scripts will first resolve to project node_modules

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed that part now @ajitbohra, thanks for the feedback 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I've removed that part now @ajitbohra, thanks for the feedback 👍

I think he meant to change "wp-env": "packages/env/bin/wp-env" to "wp-env": "wp-env".

We still need that there otherwise npm run won't resolve the command. In Gutenberg, we use a local path because we want to use the checked-out version of the package for development.


`wp-env` requires Docker to be installed. There are instructions available for installing Docker on [Windows 10 Pro](https://docs.docker.com/docker-for-windows/install/), [all other versions of Windows](https://docs.docker.com/toolbox/toolbox_install_windows/), [macOS](https://docs.docker.com/docker-for-mac/install/), and [Linux](https://docs.docker.com/v17.12/install/linux/docker-ce/ubuntu/#install-using-the-convenience-script).

After confirming that Docker is installed, you can install `wp-env` globally like so:
`wp-env` is also distributed through NPM (Node Package Manager), so Node.js and NPM are required. The latest LTS version of Node.js is used to develop `wp-env` and is recommended.
Copy link
Contributor

Choose a reason for hiding this comment

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

We require Node because wp-env is written in Node. Saying its because its distributed through NPM is kind of ambivalent.

Copy link
Member

Choose a reason for hiding this comment

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

There was an issue a couple weeks ago where a user ran wp-env but their node version was much older than what we support. I think this change is trying to clarify our supported node version for people consuming wp-env from outside the Gutenberg repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but the wording feels odd.

wp-env is also distributed through NPM (Node Package Manager), so

That can be omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can simplify, though for someone reading this on Github, it feels unusual to progress to the first commands that use NPM, without mentioning it as a prerequisite.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are still mentioning it right?

@talldan
Copy link
Contributor Author

talldan commented Apr 24, 2020

Thanks for all the feedback, I've made the suggested changes.

@talldan talldan merged commit d6a922d into master Apr 24, 2020
@talldan talldan deleted the update/env-package-docs branch April 24, 2020 03:49
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 24, 2020
$ npm i @wordpress/env --save-dev
```

Then modify your package.json and add an extra command to npm `scripts` (https://docs.npmjs.com/misc/scripts):
Copy link
Member

@gziolo gziolo Apr 24, 2020

Choose a reason for hiding this comment

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

You still should be able to run npx wp-env at this point. As soon as the binary is installed in node_modules/bin it will be executed from there.

I'm using this trick to test @wordpress/create-block locally:
npx wp-create-block

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that would work, but why assume they have npx? Having a script is sort of the standard way of calling local packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] Env /packages/env [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wp-env requires node v11, not mentioned anywhere
5 participants