Scripts: Remove legacy env scripts and packages.#22953
Conversation
|
Size Change: +171 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
noahtallen
left a comment
There was a problem hiding this comment.
Nice! I'm not aware of anything else that needs to be removed, so this looks pretty solid to me 👍 I'd personally like to see a deprecation message, since I'm not sure we did that yet for the tool. (especially since it can be used outside of gutenberg)
noahtallen
left a comment
There was a problem hiding this comment.
it's probably worth getting feedback from some more people (maybe @youknowriad & @gziolo), but this looks good to me 👍
gziolo
left a comment
There was a problem hiding this comment.
Great work everyone. It was quite a journey but it’s so nice to see it all nicely structured and working pretty well 🎉
Would it be a good time to include wp-env in Create Block tool? I guess having it behind the flag could be a great start that opens doors for further exciting improvements for block developers.
| ### Bug fix | ||
|
|
||
| - Update webpack configuration to not run the Sass loader on CSS files. It's now limited to .scss and .sass files. | ||
| - Update webpack configuration to not run the Sass loader on CSS files. It's now limited to .scss and .sass files. |
There was a problem hiding this comment.
It looks like you use code auto-formatting that adds unnecessary changes. It feels like we should enable Prettier or something similar for markdown files to stop seeing the same file being converted back and forth.
@mkaz you started on linter for markdown, have you had a chance to look at formatters?
There was a problem hiding this comment.
This is my local prettier running on markdown files.
We can use prettier for almost all file types now.
| "playground:dev": "echo \"Please use storybook:dev instead.\"", | ||
| "preenv": "npm run check-engines", | ||
| "env": "wp-scripts env", | ||
| "wp-env": "packages/env/bin/wp-env" |
There was a problem hiding this comment.
Have you considered making “npm run env” the primary way to reference it everywhere?
There was a problem hiding this comment.
That would be confusing because the binary is wp-env. It's better to be aligned with what people use when invoking a global installation.
There was a problem hiding this comment.
I’m not so convinced that the name of binary file should be the main reason to decide. If they know about the underlying tool, they can always call npx wp-env that is shorter.
env is simply shorter and I’m sure that wp-env is something that is largely unfamiliar for folks that contribute.
Anyway, whatever you pick is fine here 🤷♂️
There was a problem hiding this comment.
People have already been using npm run wp-env, and it's in all the docs.
packages/scripts/scripts/env.js
Outdated
| ); | ||
| process.stdout.write( | ||
| chalk.blue( | ||
| '\nSee: https://developer.wordpress.org/block-editor/packages/packages-env/\n' |
There was a problem hiding this comment.
Npm link would be better because it’s always up to date. The handbook is pinned to the last major WordPress release so might be up to a few months old like it’s the case now with WordPress 5.4.
There was a problem hiding this comment.
It'd be nice if the handbook was versioned and we could link to the latest/next version.
packages/scripts/CHANGELOG.md
Outdated
| - The default babel configuration has changed to only support stage-4 proposals. This affects the `build` and `start` commands that use the bundled babel configuration; if a project provides its own, this change doesn't affect it. [#22083](https://github.com/WordPress/gutenberg/pull/22083) | ||
| - The bundled `wp-prettier` dependency has been upgraded from `1.19.1` to `2.0.5`. Refer to the [Prettier 2.0 "2020" blog post](https://prettier.io/blog/2020/03/21/2.0.0.html) for full details about the major changes included in Prettier 2.0. | ||
| - The bundled `eslint` dependency has been updated from requiring `^6.8.0` to requiring `^7.1.0`. | ||
| - The `env` family of scripts has been removed. |
There was a problem hiding this comment.
It would be great to include detailed info about the alternative so people don’t miss the opportunity to migrate. A link to this PR might be also helpful for those who want to explore this change further.
This PR removes the deprecated and now unused
envfamily of scripts and updates the relevant documentation.