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

Rename @wordpress/env to wp-env #21854

Closed
wants to merge 1 commit into from

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Apr 24, 2020

Renames the @wordpress/env package to wp-env so that the package name matches the binary name. This means users can run wp-env using npx wp-env without having to specify -p @wordpress/env which is often forgotten.

If this PR is merged then the next steps are to:

  1. Publish Lerna packages so that wp-env is added to the npm registry.
  2. Run npm deprecate on @wordpress/env with a deprecation message that instructs users to use npm install wp-env instead.

An alternative approach here would be to move wp-env into its own GitHub repository and out of the Lerna monorepo.

@noisysocks noisysocks added npm Packages Related to npm packages [Tool] Env /packages/env labels Apr 24, 2020
@github-actions
Copy link

github-actions bot commented Apr 24, 2020

Size Change: -18.3 kB (2%)

Total Size: 817 kB

Filename Size Change
build/annotations/index.js 3.62 kB +3 B (0%)
build/api-fetch/index.js 4.08 kB +3 B (0%)
build/autop/index.js 2.82 kB -3 B (0%)
build/block-directory/index.js 6.23 kB -6 B (0%)
build/block-editor/index.js 106 kB -118 B (0%)
build/block-editor/style-rtl.css 10.2 kB -14 B (0%)
build/block-editor/style.css 10.2 kB -14 B (0%)
build/block-library/editor-rtl.css 7.03 kB -19 B (0%)
build/block-library/editor.css 7.03 kB -18 B (0%)
build/block-library/index.js 112 kB +82 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB +1 B
build/block-serialization-spec-parser/index.js 3.1 kB -1 B
build/blocks/index.js 48.1 kB -6 B (0%)
build/components/index.js 179 kB -18.6 kB (10%) 👏
build/compose/index.js 6.66 kB +1 B
build/core-data/index.js 11.4 kB +11 B (0%)
build/data/index.js 8.42 kB -6 B (0%)
build/date/index.js 5.47 kB +3 B (0%)
build/dom-ready/index.js 568 B -1 B
build/edit-navigation/index.js 3.54 kB -2 B (0%)
build/edit-post/index.js 27.8 kB +46 B (0%)
build/edit-site/index.js 11 kB +30 B (0%)
build/edit-widgets/index.js 8.33 kB -5 B (0%)
build/editor/index.js 43.4 kB +43 B (0%)
build/element/index.js 4.65 kB -2 B (0%)
build/format-library/index.js 7.63 kB +314 B (4%)
build/i18n/index.js 3.56 kB +1 B
build/is-shallow-equal/index.js 710 B -1 B
build/list-reusable-blocks/index.js 3.12 kB +1 B
build/media-utils/index.js 5.29 kB +2 B (0%)
build/notices/index.js 1.79 kB +1 B
build/nux/index.js 3.4 kB -3 B (0%)
build/primitives/index.js 1.5 kB +7 B (0%)
build/redux-routine/index.js 2.84 kB -1 B
build/rich-text/index.js 14.8 kB +8 B (0%)
build/server-side-render/index.js 2.68 kB +6 B (0%)
build/url/index.js 4.02 kB +2 B (0%)
build/viewport/index.js 1.84 kB +1 B
build/wordcount/index.js 1.18 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-library/style-rtl.css 7.14 kB 0 B
build/block-library/style.css 7.14 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom/index.js 3.1 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/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/style-rtl.css 5.26 kB 0 B
build/edit-site/style.css 5.25 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/escape-html/index.js 733 B 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/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 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/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/priority-queue/index.js 789 B 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@@ -1,5 +1,5 @@
{
"name": "@wordpress/env",
"name": "wp-env",
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm assuming this is what Lerna looks at when publishing the package.

@@ -1582,5 +1576,11 @@
"slug": "packages-wordcount",
"markdown_source": "../packages/wordcount/README.md",
"parent": "packages"
},
{
"title": "@wordpress/wp-env",
Copy link
Member Author

Choose a reason for hiding this comment

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

The tool that automatically generates this is incorrectly inserting the @wordpress/ prefix.

Copy link
Member

Choose a reason for hiding this comment

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

Is it hard to fix? I tend to disagree with my previous comment to keep the package that is an alias outside of Gutenberg 😂

@noisysocks
Copy link
Member Author

An alternative approach here would be to move wp-env into its own GitHub repository and out of the Lerna monorepo.

Pros:

  • Can release new versions of wp-env whenever we want.
  • Can keep the @wordpress/ convention in the monorepo.

Cons:

  • Don’t get to use Gutenberg’s continuous integration.
  • Extra confusion for folks when submitting issues.

@gziolo
Copy link
Member

gziolo commented Apr 24, 2020

Wouldn’t it be easier to improve docs rather to go through this root that will deprecate the current package that folks already use and they might have installed globally? 😅

Aside: all packages are grouped under WordPress organization and this would introduce an exception. It probably wouldn’t be listed at https://www.npmjs.com/org/wordpress as well.

@gziolo
Copy link
Member

gziolo commented Apr 24, 2020

Can release new versions of wp-env whenever we want.

You can always cherry-pick PR to wp/trunk branch and do the release there if the feature or bug fix is very important. We did it in the past several times 😃

@noisysocks
Copy link
Member Author

Wouldn’t it be easier to improve docs rather to go through this root that will deprecate the current package that folks already use and they might have installed globally? 😅

#21809 does this. But I think there's value in renaming the package on top of that. There was some discussion in this Slack thread. I think the most compelling reason to do this is so that nobody else can register wp-env and have their code run when folks accidentally type npx wp-env instead of npx wp-env -p @wordpress/env.

@gziolo
Copy link
Member

gziolo commented Apr 24, 2020

Can we maybe add another package outside of Gutenberg that aliases the package then?

It’s too late for wp-scripts: https://www.npmjs.com/package/wp-scripts

@noisysocks
Copy link
Member Author

Is that possible? 🙂

@gziolo
Copy link
Member

gziolo commented Apr 24, 2020

Is that possible? 🙂

Good question, in theory if you add a package that has @wordpress/env as a dependency the binary would get installed so it should work. I never tried it though 😜

Comment on lines +11 to 12
$ npm -g i wp-env
$ wp-env start
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be switched back to npx wp-env start if/when the package is published. Is it worth doing that now or waiting?

@noisysocks noisysocks force-pushed the update/rename-@wordpress/env-to-wp-env branch from 9e7752d to c25755e Compare April 27, 2020 05:17
Rename the @wordpress/env package to wp-env so that the package name
matches the binary name. This means users can run wp-env using `npx
wp-env` without having to specify `-p @wordpress/env` which is often
forgotten.
@noisysocks noisysocks force-pushed the update/rename-@wordpress/env-to-wp-env branch from c25755e to 002307b Compare April 27, 2020 05:27
@noisysocks noisysocks marked this pull request as ready for review April 27, 2020 05:47
@noisysocks
Copy link
Member Author

noisysocks commented Apr 27, 2020

Travis CI is now happy with this PR! I've marked it as ready for review.

My preference is to rename @wordpress/env to wp-env in the monorepo, pull wp-env out into a new WordPress/ GitHub repository, look into renaming the binary to be @wordpress/env, or simply do nothing and leave things as are.

Creating a new wp-env package which aliases @wordpress/env feels to me like it's against the spirit of npm's Acceptable Content rule:

Content that exists only to "reserve" a name, whether a package name, user name, or organization name. The Dispute Policy governs how npm handles such cases of "squatting".

What do you think @epiqueras?

@youknowriad
Copy link
Contributor

Can you explain why you want to rename the package? Is it because of

npx wp-env

I don't find the following to be the bad

npx @wordpress/env

I personally like the consistency across our packages. Another possibility is @wp/env or @wp/* for all packages.

@noahtallen
Copy link
Member

noahtallen commented Apr 27, 2020

I'm not a big fan of changing the package name -- I think it could (?) create more issues than it solves. We've already managed to get a lot of folks onboard with this tool, and I still get notified about comments on the original announcement post every week. So I mostly worry that this change will unnecessarily confuse everyone who has already gotten on board.

I guess I don't see the benefits of a rename as being that good for users. For example:

An alternative approach here would be to move wp-env into its own GitHub repository and out of the Lerna monorepo.

Pros:

Can release new versions of wp-env whenever we want.
Can keep the @wordpress/ convention in the monorepo.

Cons:

Don’t get to use Gutenberg’s continuous integration.
Extra confusion for folks when submitting issues.

I feel like the con of confusion is the main thing affecting users. I don't think the other items have a big affect on them.

@noahtallen noahtallen closed this Apr 27, 2020
@noahtallen noahtallen reopened this Apr 27, 2020
@noahtallen
Copy link
Member

sorry, accidentally clicked the close button instead of the comment button 😅 😅

@noisysocks
Copy link
Member Author

Thanks for the thoughts! I agree with the concerns raised.

I'll explore if we can rename the binary to @wordpress/env so that this tool can be run using npx @wordpress/env.

If that isn't feasible, I'll do nothing and npx -p @wordpress/env wp-env or npm install -g @wordpress-env; wp-env will remain the way to do this.

@noisysocks noisysocks closed this Apr 29, 2020
@aristath aristath deleted the update/rename-@wordpress/env-to-wp-env branch November 10, 2020 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Packages Related to npm packages [Tool] Env /packages/env
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants