Opened 6 weeks ago
Last modified 4 weeks ago
#51966 new enhancement
npm/grunt watch/build task names are inconsistent and unintuitive
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 5.1 |
Component: | Build/Test Tools | Keywords: | has-patch 2nd-opinion |
Focuses: | javascript, docs | Cc: |
Description
#43055 / #44492 made a lot of changes to the typical dev workflow, and I often have trouble getting the right watch or build command going, because they don't seem intuitive or consistent to me.
The dev
flag feels vague, and doesn't describe what it actually does (builds into src/
instead of build/
), the npm
commands aren't always internally consistent, or consistent w/ the corresponding grunt
commands. They're also syntax differences between npm
and grunt
which need to be memorized.
Some examples:
- There's
npm run build
andnpm run build:dev
, but the corresponding commands arenpm run watch
andnpm run dev
, instead ofnpm run watch:dev
. npm run build
wrapsgrunt build
, butnpm run dev
wrapsgrunt watch --dev
; there is no correspondinggrunt dev
task.- running
npm run watch --dev
will rungrunt watch
(intobuild/
) instead ofgrunt watch --dev
(intosrc/
). That's becausenpm
uses-- --
to pass a flag to the proxied command, but grunt uses--
.
The commands aren't clearly documented in the Handbook or readme file, so I have to read the package.json
or Gruntfile.js
, and try to memorize the inconsistencies.
Ideas
I think it'd help to:
- Add clear documentation about the purpose of the
src/
andbuild/
folders, the pros/cons of running from each, and the commands needed for each scenario. - Rename the commands/flags to be self-documenting and consistent.
I'll open a PR with a rough implementation of those as a starting point.
Change History (8)
This ticket was mentioned in PR #798 on WordPress/wordpress-develop by iandunn.
6 weeks ago
- Keywords has-patch added
#2
@
5 weeks ago
- Keywords 2nd-opinion added
I often have trouble getting the right watch or build command going, because they don't seem intuitive or consistent to me.
Same here :)
Frankly I'm thinking the whole "build in one of two places" setup is quite confusing especially for new contributors. Even more when trying to run the tests. Where are they actually running from, build
or src
, or... sometimes from here, other times from there..? :)
Even more confusing/error prone is when doing svn up
followed by grunt build
. Then the old, outdated build files are still left in /src
. So if you happen to run WP or tests from /src
you end up with a mixture of old "built" files (js, css, and some php) mixed with new source files. A mess...
The patch at https://github.com/WordPress/wordpress-develop/pull/798 looks good and makes it easier/better. The inline docs will help too. Thinking it shouldn't stop there though.
Seems the best way to fix the above "mess" would be to always build in both places, build
and src
. That may be a bit slower, but will simplify a lot of things and prevent a lot of confusion and headaches. Actually it looks like it won't be slower, can build to src
and then copy all built files to build
. Looking at Gruntfile.js this is pretty easy to do there but will need to fix/tweak Webpack a bit.
Do you think this is a good idea? If we do that, then we can change/simplify/deprecate a lot more of the build workflow and commands, and make it simple again.
#3
@
5 weeks ago
I love that! Having just grunt watch
and grunt build
(and npm run build
, npm run watch
) would really simplify this.
Leaving 2nd-opinion
for a few others to chime in, since it'd be a big change
cc @netweb, @desrosj, @omarreiss, @atimmer, @johnbillion
#4
@
5 weeks ago
Recalling our thinking
I spend some time remembering what we were trying to do. The reason for building to src
was that we identified that for some WordPress contributors, having to run a watch task to make PHP changes would be a non-starter. I now think that we made a mistake, but that it might have been right within the thinking of that time. Having build
& src
is too confusing, I even have trouble recalling how it works, which is not a good sign.
So fundamentally, a proper solution would have one folder to run from.
Different approaches
This 'proper solution' results in a philosophical conflict. The WordPress PHP has long had a philosophy of zero-installs. Cloning the repository means it works. The JavaScript world has long had a philosophy of having a build step before it works. I used to be a sole proponent of lean and mean repositories. Only commit the absolute necessities and have a build step producing pristine artifacts that can be run.
My current understanding is that both are a tradeoff with different qualities. The package manager Yarn has recognised this and supports zero-installs very well now: https://yarnpkg.com/features/pnp#fixing-node_modules. "The generated .pnp.js file can be committed to your repository as part of the Zero-Installs effort, removing the need to run yarn install in the first place."
It is not for me to say, but I have a feeling that zero-installs fit better with WordPress's philosophies. Zero-installs are decidedly more accessible as they require less tooling on the part of a new contributor. But it might be that that ship has sailed with Gutenberg.
Band-aid
Having said all that, I think @azaozz's proposal is a proper band-aid as long as the performance is acceptable. So 👍🏻 for simply doing that.
#5
@
5 weeks ago
That's really insightful, thanks! I hadn't heard about Zero Installs before, but I think that's a really interesting idea. I'm wondering if something like this would work:
- Remove the
build/
folder entirely. - All
.php
files insrc/
stay the same. There's no longer a need to copy them over tobuild/
. - Keep
src/js
the way it is - (optionally) create a
src/sass
folder (cc @helen, @melchoyce) src/js
files that don't require any preprocessing -- likedashboard.js
-- could be removed fromsrc/js
. Then changes to them could be made directly tosrc/wp-admin/js/bashboard.js
without having to worry about a build step.- We'd add built files like
src/wp-admin/js/foo.js
to.gitignore
, so that devs don't have to locally manage duplicategit add
's etc. - We'd setup a A GitHub action to automatically compile & commit the built files to the repo on a
post-push
hook, like @ryelle did for TwentyTwentyOne.
So the typical dev workflow would be:
- If you're making changes to processed JS or SASS files, then you
npm run watch
. Otherwise, you don't have to run any watch task at all. - Make changes to the files
- If they're processed JS/SASS files,
watch
would compile them fromsrc/js
intosrc/wp-admin/js
,src/wp-includes/js
etc. git add
the PHP,src/js
, orsrc/sass
files. The built files won't be tracked locally, so you won't get bugged by them.git push
- If any
src/js
orsrc/sass
files were pushed, the GH action would compile and commit them to the PR.
Committers could wget {PR URL}.diff
, and commit that. It'd include both src/js
and src/wp-admin/js
files.
The release process would bundle up all of src/
but ignore src/js
and src/sass
.
#6
@
5 weeks ago
(optionally) create a src/sass folder
The color schemes use sass, but we can leave the current structure (wp-admin/css/colors/*.scss
) in place. They're already built in src if you use the dev commands. Currently there's no plan to use Sass anywhere else, if anything we're moving away from it (unless Gutenberg has other plans for core). The PostCSS process will probably be expanded on, but I don't think that means we need a separate CSS source folder.
It looks like the last two default themes have also come into core with build processes, so we might want something like yarn's workspaces to centrally manage building those, too.
This is just a rough & untested sketch of what it could look like, to get feedback on the general concept.
Trac ticket: https://core.trac.wordpress.org/ticket/51966