Reorganize Core JS / introduce build step

[omarreiss]

This ticket aims to introduce a build step and better code management to WordPress core JavaScript. The current way the JavaScript is organized raises some concerns and needs an update to bring it up to par with current day processes for JS development. The last few months I've been busy figuring out what the concerns are with regard to JS code organization in WordPress, what a good process for modernizing it would be and what we need to change in order to facilitate such a process and be ready to include ie. Gutenberg into WordPress. In this project I've consulted with ​Gary Pendergast, ​Adam Silverstein, ​Riad Benguella and ​Andrew Duthie in #core-js and on WCUS and ​Anton Timmermans and ​Herre Groen at Yoast HQ.

A build step for WordPress core developent, what would change?

This patch will introduce a build step to WordPress core development. Once this gets committed, it will no longer be possible to run WordPress from the src directory. To make this transition easier for developers, we've added a special page that people running WordPress from source will start seeing:

As the screen says, npm install && grunt build will be enough to trigger a build. The JavaScript in the source has been organized to meet the following concerns.

Code organization concerns

1. Need to maintain backwards compatibility while providing flexibility.

We've discussed we want all enqueued scripts to still be built and present in the respective wp-admin/js and wp-includes/js. As they are there right now, they are really hard to manage. The way the code is organized is very hard to understand and it is very hard to refactor anything. To break out of the current "mess", it would be nice to have all JS source in one directory, from which it is copied or bundled to its legacy location. It's fully backwards compatible but adds an important first layer of flexibility that'll help advance JS development in core.

2. Need to separate scripts that end up in WordPress from source code and provide freedom in working with the source and organizing it in the best way.

Now that we have all the JS together in one directory, it will still be a mess to change things or split things up. It would not be transparent what would end up being enqueued and what is the source for those enqueues. In an ideal world, enqueued scripts are nothing more than a sort of manifests that load source code, initialize objects and perhaps set a few hooks. They would ideally not contain actual behavior. As we currently don't have a way to distinguish between enqueues and source code, basically everything right now is an enqueue. With the media library we've tried to demonstrate how this could work when you separate the source away. The media library then doesn't assign itself to the wp global (see ​src/js/media/models.js example). Instead this is then done in the enqueue (​src/js/_enqueues/wp/media/models.js example). In fact It's practically all the enqueued script does.

3. Need to organize the enqueued scripts in a better way.

In the current setup, vendor scripts are mixed with self contained utility or lib scripts, deprecated scripts that are no longer in use and "other" scripts that are so all over the place that they are hard to classify. It's very hard to understand what's going on from looking at the directory structure. A large part of scripts simply assign something to the wp namespace. Why not have a directory structure for scripts which just follows the namespace? It also immediately becomes clear what should be the responsibility of those files, namely to assign modules to the namespace. The modules themselves could be moved to the src/js root and split up easily if deemed useful. This way it will be easy to explain the upgrade path towards a more modern code base. Things can simply be extracted / abstracted in multiple iterations.

Not every script assigns something to the wp namespace. We've come up with a ​directory structure that seems useful to us, but anything is possible really. A very nice advantage is that deprecated and vendor scripts become much easier identifiable. This what it could look like:

4. Need to manage vendor scripts better

In the current setup. Vendor scripts that exist as NPM packages are actually shipped with WordPress and managed with a sort of copy/paste system. Managing these with NPM and having a copy task or Webpack move them into the right place makes so much more sense. Most of the NPM packages simply ship with the dist files, so we only have to copy them to the right location. But now we actually manage those with NPM and they are listed in the package.json, which to me seems like a huge improvement.

Sometimes vendor scripts need to be built after install. This holds true for maybe 2 or three dependencies. This can simply be ​part of the build task.

Some vendor scripts cannot be managed with NPM. I've moved them to a separate ​src/js/_enqueues/vendor directory and added a ​copy task to the Gruntfile to copy these files over to their original location.

5. Need to install WordPress packages and assign them to a global.

​WordPress Packages is an initiative to extract general purpose WP libraries to separate NPM packages, so they can also be used outside of the scope of WordPress. The first example of this is wp-includes/wp-a11y.js. It's easy to configure Webpack to automatically generate that file based on the ​@wordpress/a11y package. This was the chore that lead to this undertaking. Technically it's not a very hard thing to do, but I'd like to be as forward compatible as possible, so that's why I decided to zoom out a bit.

Webpack simply provides the ​library option for this kind of thing.

entry: {
        'src/wp-includes/js/wp-a11y.js': ['@wordpress/a11y']
},
output: {
        filename: 'src/wp-includes/js/wp-a11y.js',
        library: [ 'wp', 'a11y', 'speak' ],
}

6. Allowing a build step.

At WCUS, we pretty much agreed a build step is a necessity in modern JS development. While we can do much to help onboard new developers to get used to this, it simply makes sense to run WP from the build directory, no longer directly from source. We definitely intend to create good documentation for this that is also referenced in the notice showed earlier.

7. Managing packages under WP source.

At WCUS, it was pretty much decided WP packages should be managed in WP core after all. This setup can easily facilitate that. I'd propose to simply have a packages directory in src/js where we can continue managing them with ​lerna. This will also make it incredibly easy to publish packages from WordPress core to NPM, which could help us increase our impact in open source even further.

---

Testing

I ran a diff to compare the build that this thing generates with the current build on master. There are still a few minor differences:

➜  wp diff -qr compare/build mirror/build
Files compare/build/wp-admin/css/common-rtl.css and mirror/build/wp-admin/css/common-rtl.css differ
Files compare/build/wp-admin/css/common-rtl.min.css and mirror/build/wp-admin/css/common-rtl.min.css differ
Files compare/build/wp-includes/js/backbone.min.js and mirror/build/wp-includes/js/backbone.min.js differ
Files compare/build/wp-includes/js/hoverIntent.js and mirror/build/wp-includes/js/hoverIntent.js differ
Files compare/build/wp-includes/js/hoverIntent.min.js and mirror/build/wp-includes/js/hoverIntent.min.js differ
Files compare/build/wp-includes/js/imagesloaded.min.js and mirror/build/wp-includes/js/imagesloaded.min.js differ
Files compare/build/wp-includes/js/jquery/jquery-migrate.js and mirror/build/wp-includes/js/jquery/jquery-migrate.js differ
Files compare/build/wp-includes/js/jquery/jquery.color.min.js and mirror/build/wp-includes/js/jquery/jquery.color.min.js differ
Files compare/build/wp-includes/js/jquery/jquery.js and mirror/build/wp-includes/js/jquery/jquery.js differ
Files compare/build/wp-includes/js/masonry.min.js and mirror/build/wp-includes/js/masonry.min.js differ
Files compare/build/wp-includes/js/media-audiovideo.js and mirror/build/wp-includes/js/media-audiovideo.js differ
Files compare/build/wp-includes/js/media-audiovideo.min.js and mirror/build/wp-includes/js/media-audiovideo.min.js differ
Files compare/build/wp-includes/js/media-grid.js and mirror/build/wp-includes/js/media-grid.js differ
Files compare/build/wp-includes/js/media-grid.min.js and mirror/build/wp-includes/js/media-grid.min.js differ
Files compare/build/wp-includes/js/media-models.js and mirror/build/wp-includes/js/media-models.js differ
Files compare/build/wp-includes/js/media-models.min.js and mirror/build/wp-includes/js/media-models.min.js differ
Files compare/build/wp-includes/js/media-views.js and mirror/build/wp-includes/js/media-views.js differ
Files compare/build/wp-includes/js/media-views.min.js and mirror/build/wp-includes/js/media-views.min.js differ
Files compare/build/wp-includes/js/twemoji.js and mirror/build/wp-includes/js/twemoji.js differ
Files compare/build/wp-includes/js/underscore.min.js and mirror/build/wp-includes/js/underscore.min.js differ
Files compare/build/wp-includes/js/zxcvbn.min.js and mirror/build/wp-includes/js/zxcvbn.min.js differ
Files compare/build/wp-includes/version.php and mirror/build/wp-includes/version.php differ

For most of those there is a valid explanation. The media files have been slightly altered to fit the new setup. Some files are now copied over from node_modules and don't end in a newline. I still have to do a little bit of investigation here.

[omarreiss]

If you wish to checkout the branch for yourself and play around with it, this is what I based the patch on: ​https://github.com/Yoast/wordpress-develop-mirror/tree/fresh-js-src-move

[omarreiss]

This ticket was discussed today in #core-js on WordPress Slack, see ​https://wordpress.slack.com/archives/C5UNMSU4R/p1515505132000043

The main concern that was raised was the transition problems that might occur in not being able to run WordPress from the src directory anymore. I took 3 options from the :

1) Stop allowing running from /src and only build to /build.
2) Allow running from /src after a build step but .gitignore the built files.
3) Allow running from /src and commit the latest stable version of the built files in /src.

The general consensus seems to be that we need to have a transition period in which we start out with option 3 and after some time transition to option 2 and/or option 1. I will do another iteration to make this possible.

I would personally be in favor of gitignoring the built files from the start, because of the confusion that comes with having a set version of build files in the source. This can lead to a myriad of issues that I'd like to avoid. If we choose option 2 as the initial approach, the configuration / dev workflow impact would be minimal, while the gain would still be complete.

[omarreiss]

To share a little more context. Here's an overview of the directory structure I've devised and the idea behind it.

src/js				| All the JavaScript source.
├── _enqueues			| Any script that ends up being enqueued.
│   ├── admin 			| Procedural scripts ran in the admin.
│   ├── deprecated		| All deprecated scripts.
│   ├── lib			| All standalone lib scripts.
│   ├── vendor			| All 3rd party deps that can't be managed with NPM.
│   └── wp			| All scripts that assign something to the wp namespace.
│       ├── customize		| Anything under wp.customize.
│       ├── editor		| Anything under wp.editor.
│       ├── media		| Anything under wp.media.
│       ├── utils		| Anything under wp.utils.
│       └── widgets		| jQuery widgets on the wp namespace.
└── media			| The media library.

As I see it, most of the future source will live outside of the _enqueues directory. One of the things this would enable us to do is to extract all of the business logic from the customizer into a modular library. This is something that can be done very easily in this structure and will make it much easier for us to get started once we move to the customizer / Gutenberg focus. The only thing that would remain in the _enqueues would be things like global assignments. An alternative approach is to not have these at all anymore and just configure Webpack to do that, but that is something we can also decide in a later stage. The important thing is that this structure makes it possible.

[netweb]

Moving to 5.0 for higher visibility

[jorbin]

Thanks for working on this @omarreiss! I fully support us doing this, but have a handful of thoughts and questions before I think it should be committed to core. I still need to review the actual code, but here are some thoughts on the idea and process for proceeding.

1) We should add file mapping to grunt-patch-wordpress so that we don't invalidate every single patch on trac that has JS in it.

2) Should we also modify the organization of qunit tests? The standard that we have used there is for test file names to match the path and name of files being tested.

3) We should come up with a communication plan for this. I think something along the lines of:

• announcing it in dev chat the week this will land
• Have a make/core post ready to go live within a few minutes of this landing in core

4) We should have some documentation about how to patch libraries that we load and also how to test those patches. npm link can be a new barrier to testing changes for many developers.

5) If we start relying on more and more external JS, I would love to come up with some philosophy points around it. In general, we should be relying on projects that have a strong commitment to backward compatibility.

6) as far as transitioning, I lean towards going all in and communicating it loud. This may also require working with maintainers of things such as VVV and other common dev platforms so that they are ready for the change.

[omarreiss]

@jorbin thanks a lot for these points!

1) Absolutely, will look into it!
2) Yes, was planning on this, but forgot about it again. I think we should. Will do the work.
3) I can definitely prepare a make/core post based on everything I've written up so far. I think @pento or @adamsilverstein would be likely candidates to eventually commit this.
4) Absolutely, I will work on docs explaining the process for managing dependencies, the build process, the directory structure and a process for extracting code from existing _enqueues scripts for reuse (some kind of boyscout principle). Then I think it would be good to have a README.md in src/js referencing this documentation.
5) I can add a draft for this to the dependency management documentation. We can always figure out later where this should end up eventually.
6) I would be in favor of that. When I discussed this with @pento he was also thinking along those lines. The most compelling argument I read against only running from build is that PHP changes will also need to be built in order to take effect. I can imagine that to be an undesireable barrier. There is also something to say for /build not really being a development build, but more of a distribution artifact. To me it seems like a good balance to allow building the JS to src as well but just adding the JS build dirs to .gitignore so that they don't get included in the source. That's still pretty much all-in if you ask me and makes the impact on dev platforms like VVV minimal. They basically only need to add npm install && grunt build to their provision scripts. In other words: maximum gain with almost zero cost.

[pento]

Nice work on this, @omarreiss. You've done a great job on this reorg, I think it has the potential to set WordPress up for a much nicer experience for contributors.

I'd like to chat about why I think this change is a a good idea.

The Problems

I've been reflecting on this issue since our discussions at WCUS, and I think it comes down to three issues that we're facing.

1. The experience of contributing to WordPress is awful. There's a high barrier of entry just to get some sort of a local development install running, it takes way too much effort to maintain, and it's really hard to collaborate. Even if you manage to produce a patch, it feels like you're throwing it into the void.

2. Adding a build step to the WordPress development flow is unavoidable. As we move towards our JS future, it's going to be necessary to make that move, Gutenberg is proving to be a useful testing ground for playing with build processes.

3. JS build tools are nearly as bad as WordPress, in terms of the ease of someone being able to contribute to a JS project. They certainly don't tackle any of the hard problems, like setting up complete local dev environment. (If you're talking to an API to store your data, and not setting up a local copy of that API server and corresponding database, your dev environment is not complete.)

The Solutions

With those problems in mind, I think there are a few promising options to solve them.

On the topic of where to build things to, I find myself leaning towards option 1, though I can't entirely dismiss option 2. Option 3 is completely out, I don't believe there's anything but pain and maintenance headaches down the road of committing built files to the source repo. Option 2 appeals, as the idea of allowing PHP changes without a build step is nice. That said, I also think it opens us up to a potentially confusing development experience, as we'll have committable PHP files living next to uncommittable JS files. Option 1 requires a build step for PHP changes, but it gives us the cleanest separation between development code and built code, even if the PHP is never really "built", as such.

I have a love/hate relationship with VVV. I love it when it works, I hate whenever I have to setup a new computer, because the VVV part is so often the most painful. I believe we can do a lot better than that, and a lot better than what the current JS process looks like. Gutenberg currently makes limited use of Docker to provide a simple testing environment, I've been playing around with ​expanding that to include running PHPUnit tests, and generally having a full development environment created automatically. That PR is obviously a little rough around the edges, but I think it points in a good direction - we can easily automate setting up the entire development environment, and it's not difficult for us to detect when something is wrong, and give useful, contextual information for a contributor to get their development environment running again.

Finally, (and this is more of a long term goal), I would be exceedingly happy if my development experience never involved opening a terminal window. App IDEs have been working on making their development experience not suck for many years, and they're a long way ahead of where the JS world is now. But there's a lot we can learn from them, and I think one of the most important lessons is that developing entirely in a GUI is an inherently better experience than typing a command into a terminal.

What's Next?

Some of the stuff above is beyond this ticket, but it gives some context for why I think this is a necessary change, that on the face of it adds some complexity to the WordPress development experience, but actually sets us up for making the experience awesome.

As @jorbin has mentioned, there are certainly some housekeeping issues to sort out before this can be committed. I agree that a make/core post is important, I'd be inclined to post it before the change is committed, not only so that people are prepared, but so we can get a wider audience on the proposal. This is a big change for WordPress development, it's appropriate that we should be able to make a convincing argument to the wider WordPress community before it lands.

If build tools are relevant to your interest, I would very much like to see more activity on the Gutenberg tools. WordPress' future tools will be heavily influenced by what works in Gutenberg, so get in early if you want to shape what that's going to look like.

[jeremyfelt]

This is great work so far @omarreiss, thank you.

2) Allow running from /src after a build step but .gitignore the built files.

This seems like the option least likely to impact existing workflows. If you work on JS, you'll already be used to the existing process and have likely used grunt watch as part of your workflow. If you work on PHP, then nothing really changes.

1) Stop allowing running from /src and only build to /build

That said, option 1 is much cleaner and I don't think it will be too impactful to users of VVV if we switch to a "must build" workflow. We already have a build.wordpress-develop.test that's intended to use with the build/ directory and that is built with grunt by default during provisioning. We'll probably need to dust that off and make sure the docs are good.

Personally - as long as the builds triggered by grunt watch (or whatever is doing the instant builds) are quick, especially when working on files that don't require a build, then +1.

[dingo_bastard]

I never had any issues with writing patches or testing stuff on VVV, and adding one small extra command line is not that complicated.

Somehow I get the feeling that we are worried that people will be thrown off of developing or contributing because they need to use the command line (which I've grown quite fond of). Even Windows has bash built into it (granted only win10 but still) :D
We should be looking at this as an opportunity to educate people how to use new and exciting tools. Sure, npm can be fidgety, sometimes some js projects work with one version of npm & node, other with different one. But I've had same experience with ruby (where one project works on 1.9.1 and not on 2.2.2 etc), and the development on those projects when compared to WordPress is just scary awful (the builds fall apart because hey, somebody removed a gem that existed n years ago and was used in the project). With WordPress I didn't have that experience. And I've learned a lot since I've started to work with it (and still do).

Point I'm trying to make is that we should use this opportunity to create a good documentation and show people that it's not hard to use these tools. I mean everybody in JS world is using either yarn or npm. WordPress is often the first touch people have with development (usually when they have to modify themes). If we show them that they can learn to use modern tools that might inspire them to learn more by themselves and be a better developer.

[gziolo]

As the screen says, npm install && grunt build will be enough to trigger a build. The JavaScript in the source has been organized to meet the following concerns.

It can be as simple as npm install. You just need to put grunt build as postinstall npm run-script. See how it is configured in WordPress/packages: ​https://github.com/WordPress/packages/blob/master/package.json#L35.

[gziolo]

Bonus question, would it be a very bad idea to call npm install from PHP using ​shell_exec function when Node.js is installed on the machine?

[Frank Klein]

Thanks for working on this patch @omarreiss. The proposed reorganisation makes sense, and I look forward to seeing this land in Core.

On the subject of the ease of contribution, I consider that this can be solved by creating better documentation. Web development has become more complex, and there's no way to get rid of that complexity, as it is there for a reason. Let's not remain stuck in the "good old days" of "hacking" on WordPress using MAMP.

As far as Gutenberg is concerned, I do not necessarily see it as being the determining factor for how Core should handle JavaScript. There is no technical requirement for having to keep the build system that Gutenberg uses. And considering the small numbers of Gutenberg contributors, familiarity with the plugin's build process is not an argument.

[jaswrks]

@omarreiss I just took your awesome changes for a test drive, running grunt build from your mirror. I then went through some of the file differences that you listed above to see if I could help.

• jquery.js needs to have jQuery.noConflict(); appended to it.

• It looks like jquery-hoverintent has a slightly different build variation at NPM (perhaps a mistake on the part of the maintainer). I ​opened an issue here in that project's repo.

• The only difference I'm seeing in the jquery.color.min.js file is in the variables used by the minifier; e.g., function m(a,b,c) vs. function h(e,t,n). They appear to be identical otherwise.

• underscore.min.js in the NPM package has a sourcemap reference at the bottom.

• zxcvbn.min.js in the NPM package has a sourcemap reference at the bottom, and for whatever reason is missing the copyright comment at the top.

[jaswrks]

In package.json, would it be a good idea to remove ^ from any of these and force any version bump to occur as the result of an intentional hard-coded change? Seems worth considering in a project like WP core.

"dependencies": {
    "@wordpress/a11y": "0.1.0-beta.1",
    "backbone": "^1.3.3",
    "imagesloaded": "^3.2.0",
    "jquery": "^1.12.4",
    "jquery-color": "github:jquery/jquery-color#2.1.1",
    "jquery-form": "^4.2.1",
    "jquery-hoverintent": "^1.8.1",
    "jquery-ui": "github:jquery/jquery-ui#1.11.4",
    "masonry-layout": "^3.3.2",
    "twemoji": "^2.5.0",
    "underscore": "^1.8.3",
    "zxcvbn": "^4.4.1"
  }

[omarreiss]

@jaswrks thanks so much for helping out with this! I can see your point with regard to hard versions, but it would be good to have an easy way to detect available updates. Something like renovate would really help, but then we'd need to be on Github or similar. ​https://github.com/apps/renovate. On the other hand, the package-lock.json already locks dependencies to a specific versions so dependency updates don't happen involuntarily.

[dingo_bastard]

In my company, for several open source libraries and projects we maintain, we use greenkeeper ​https://github.com/apps/greenkeeper, seems similar to renovate, could be useful for dependency maintenance.

[jaswrks]

It looks like jquery-hoverintent v1.8.1 has a slightly different build variation at NPM (perhaps a mistake on the part of the maintainer). I opened an issue here in that project's repo.

The hoverIntent author just published v1.8.3 at NPM to correct the problem I noted earlier.
See the author's comments here: ​https://github.com/briancherne/jquery-hoverIntent/issues/67

[omarreiss]

Thanks @jaswrks!

A quick status update (also shared in #core-js)

• @herregroen has worked on optimizing the watch process. After the initial build he has managed to bring rebuilding down from 20 to 5 seconds. The goal is to get under 1 second.
• I am working on documentation surrounding the patch. Work in progress available here: ​https://gist.github.com/omarreiss/4d6ec6257e55ab64f189c3a1680336a8. Working from one document, but this will probably end up as multiple pages on the handbook. Would be nice if @jorbin / @adamsilverstein / @pento would be willing to give it a first look already, just to check if I am heading in the right direction.
• @jorbin gave some feedback in #core-js on my approach of the qunit tests. Will process that before I upload a new patch.

Will try to upload a new patch shortly which:

• Includes a faster grunt watch.
• Fixes inconsistencies that were still left between the builds.
• Fixes the qunit tests.
• Includes an initial draft of the documentation needed.

Once that is done, I would like to discuss the next steps needed to move this forward.

[adamsilverstein]

@omarreiss Great work here, I'm very excited to see this step forward.

I reviewed the branch in progress on your mirror and also pushed to my fork to run in Travis - noting that a bunch of tests fail after this reorgnization that we'll need to fix, I didn’t look into the reason: ​https://travis-ci.org/adamsilverstein/wordpress-develop-fork.

Overall this is looking really good, I have a few questions and comments:

• I keep a bunch of stuff in wp-content, so when my 'build' step takes a little time to copy all the files over. Maybe grunt watch should skip cleaning/copying files?

I see you have the media source files in a media folder under js, wondering if this should move down one level into a source folder so that as we add more build file folders they don't fill up the top level folder.

I'll review the documentation you linked as well, thanks!

[netweb]

Replying to jaswrks:

In package.json, would it be a good idea to remove ^ from any of these and force any version bump to occur as the result of an intentional hard-coded change? Seems worth considering in a project like WP core.

I've already started doing this from r42463 and all ^ and ~ are removed from patches yet to be committed.

Replying to omarreiss:

@jaswrks thanks so much for helping out with this! I can see your point with regard to hard versions, but it would be good to have an easy way to detect available updates. Something like renovate would really help, but then we'd need to be on Github or similar. ​https://github.com/apps/renovate. On the other hand, the package-lock.json already locks dependencies to a specific versions so dependency updates don't happen involuntarily.

The plan is to have Greenkeeper up and running on a mirror for this

[omarreiss]

Uploaded a new patch with the following fixes:

• grunt watch now under 1 second thanks to work done by @herregroen. CC: @adamsilverstein
• jQuery.noConflict() is appended to jquery.js using a file_append task. CC: @jaswrks
• grunt qunit now passes again. I've decided to run the JS from the build directory since most of those tests are integration tests anyway. CC: @jorbin @adamsilverstein
• Hardcoded version numbers for dependencies in package.json. CC: @jaswrks @netweb
• Updated jquery-hoverintent to 1.8.3 to fix inconsistencies. CC: @jaswrks

Todo (mostly documentation):

• I will create core handbook drafts to document the JS contribution process better and include philosophy points. I would like to add a src/js/README.md that references those handbook pages.
• @jorbin gave feedback on the file mapping PR: ​https://github.com/Yoast/grunt-patch-wordpress/pull/1. File mapping config should be added to the project itself. I still need to do this.

@adamsilverstein With regard to introducing an extra directory. The idea is that the source lives under src/js. We've discussed putting the modular source code in src/js/src but this felt confusing to most of us. I did add an _enqueues directory to separate unbundled modules from scripts that are loaded.

With regard to development platforms like VVV, I'd prefer those to run grunt watch internally, so a user who runs vagrant up can just edit the source and everything gets built automatically. If that is the case, it really doesn't matter anymore if we require WordPress to be run from build, since anyone using a dev platform will not be concerned with that.

@pento what do you consider to be the next steps after this?

[netweb]

Some notes regarding js-organization-2.patch​:

Firstly which this relates to the first bullet point coming up, can ​the branch for js-organization-2.patch​ be updated from master//trunk? (FYI: I'm not sure if there are potentially other issues caused by this)

• package.json is since [42460] indented using tabs (The new package-lock.json file will need to be regenerated after this is fixed and will then inherit the tabbed indentation)

• jquery-color, was there a reason to choose v2.1.1 over the latest 2.x branch v2.1.2? (​comparison here). It also looks like they are resolving the issue of the package not being published to npm in ​jquery/jquery-color/issues/84

• jquery-ui, was there a reason to choose v1.11.4 over the latest c1.12.1 which is published to npm? (​comparison here)

Pausing my review here... @omarreiss Would you prefer comments ​on the PR? I think that might be a little easier to manage feedback?

[netweb]

Is the plan to use svn mv to move the src/index.php to src/_index.php and then update the moved file or files so that file history is maintained? (I hope so)

[omarreiss]

@netweb I have tried to leave updating dependencies out of scope for this patch. The versions in package.json are the versions that are in core right now, as far as I could verify.

Is the plan to use svn mv to move the src/index.php to src/_index.php

That's absolutely a must have. Will put that on my todo list.

@omarreiss Would you prefer comments on the PR? I think that might be a little easier to manage feedback?

That would be very helpful.

PR for grunt-patch-wordpress has been opened here: ​https://github.com/WordPress/grunt-patch-wordpress/pull/54

Will await review by @netweb, process feedback and include file mapping before I upload a 3rd patch.

[netweb]

Replying to omarreiss:

@netweb I have tried to leave updating dependencies out of scope for this patch. The versions in package.json are the versions that are in core right now, as far as I could verify.

Great, I'll continue to update these as I've been doing recently and we can merge those changes here when needed

Is the plan to use svn mv to move the src/index.php to src/_index.php

That's absolutely a must have. Will put that on my todo list.

Excellent, creating a list of each of these would be beneficial, maybe even separating the patch into two patches, one patch with changes, the other patch contains file moves where there are no changes, just renamed/moved.

@omarreiss Would you prefer comments on the PR? I think that might be a little easier to manage feedback?

That would be very helpful.

Will do 👍🏼

PR for grunt-patch-wordpress has been opened here: ​https://github.com/WordPress/grunt-patch-wordpress/pull/54

Will await review by @netweb, process feedback and include file mapping before I upload a 3rd patch.

I had a quick look at that PR and is looking good, @jorbin has reviewed it and requested a few changes, I'll take another look after those changes are in.

[omarreiss]

Working towards completion:

The Grunt patch PR was merged. ​https://github.com/WordPress/grunt-patch-wordpress/pull/54. File mapping is included in the latest patch.

I went through all of the file diffs between master build directory and the build directory after patch. Verified there are no logic differences anymore. Here's the summary:

Files compare/build/wp-includes/js/backbone.min.js and mirror/build/wp-includes/js/backbone.min.js differ
└ Only difference is a sourcemap comment which was omitted in the current build.

Files compare/build/wp-includes/js/hoverIntent.js and mirror/build/wp-includes/js/hoverIntent.js differ
└ Only difference is a version number in the comment. NPM version was unauthorized before. This is fixed and the reason why the versions diff.

Files compare/build/wp-includes/js/imagesloaded.min.js and mirror/build/wp-includes/js/imagesloaded.min.js differ
└ Code is identical, was minified twice in the current build. Fixed in patch.

Files compare/build/wp-includes/js/jquery/jquery.color.min.js and mirror/build/wp-includes/js/jquery/jquery.color.min.js differ
└ Code is identical, only difference is minification variable names.

Files compare/build/wp-includes/js/media-audiovideo.js and mirror/build/wp-includes/js/media-audiovideo.js differ
Files compare/build/wp-includes/js/media-audiovideo.min.js and mirror/build/wp-includes/js/media-audiovideo.min.js differ
Files compare/build/wp-includes/js/media-grid.js and mirror/build/wp-includes/js/media-grid.js differ
Files compare/build/wp-includes/js/media-grid.min.js and mirror/build/wp-includes/js/media-grid.min.js differ
Files compare/build/wp-includes/js/media-models.js and mirror/build/wp-includes/js/media-models.js differ
Files compare/build/wp-includes/js/media-models.min.js and mirror/build/wp-includes/js/media-models.min.js differ
Files compare/build/wp-includes/js/media-views.js and mirror/build/wp-includes/js/media-views.js differ
Files compare/build/wp-includes/js/media-views.min.js and mirror/build/wp-includes/js/media-views.min.js differ
└ The way media files are built has slightly changed which results in some bundling / minification diffs.

Files compare/build/wp-includes/js/underscore.min.js and mirror/build/wp-includes/js/underscore.min.js differ
└ Only difference is a sourcemap comment which was omitted in the current build.

Files compare/build/wp-includes/js/zxcvbn.min.js and mirror/build/wp-includes/js/zxcvbn.min.js differ
└ Looks like the current file is broken. The forbidden passwords look scrambled. Fixed in patch.

This means codewise this is ready to go. Current patch enforces build step and requires to run from /build.
I contacted @tjnowell on WP Slack to discuss steps for VVV. I'd also love to know about other dev platform maintainers I should contact. It will be best if they are mostly prepared.

Will spend another half day on documentation this weekend. Would like to move to inclusion starting monday. @pento is this something you could help with?

[ocean90]

Replying to omarreiss:

Files compare/build/wp-includes/js/zxcvbn.min.js and mirror/build/wp-includes/js/zxcvbn.min.js differ
└ Looks like the current file is broken. The forbidden passwords look scrambled. Fixed in patch.

Core uses ROT13 for the dictionary, see [25156]/[25159] and ticket:31647:9. Is that still the case with your patch?

[TJNowell]

VVV issue raised ​https://github.com/Varying-Vagrant-Vagrants/VVV/issues/1447

[omarreiss]

@ocean90 thanks a lot for that context! This is indeed broken now. Will fix.

[omarreiss]

Decided to include zxcvbn manually because I can't think of a reliable way to automate the ROT13 transformation. Patch is ready now as far as I can see.

[omarreiss]

I've bundled the documentation I have written so far in a Drive folder to enable collaboration:

​https://drive.google.com/open?id=1FDD8QQ5TmydZ62nMzPb8pmiVpUSCnhsv

I would love to get feedback or contributions on those documents.

[pento]

Nice work on the docs, @omarreiss! I left a couple of smaller comments, but I generally like the direction you're going here.

On a broader topic, the "Setting up your local development environment" document is a sharp reminder that we still have a super obtuse new developer experience. The Docker-based setup scripts that Gutenberg uses are a reasonable first step towards fixing this, but it's going to need work before it can land in Core.

I don't think this ticket needs to be dependent on fully automated devenv setup being implement for Core, but it's good to keep that in mind when requiring new tools to be installed for people to develop Core.

[pento]

A few notes:

• Needs an .nvmrc file.
• node-sass is the worst, and regularly needs rebuilding.
• grunt build gives me an error when it tries to build jquery-color:

npm ERR! path /srv/www/wordpress-develop/public_html/node_modules/jquery-color/node_modules/nyc/node_modules/yargs/node_modules/cliui/package.json.3362228139
npm ERR! code ENOENT
npm ERR! errno -2
npm ERR! syscall open
npm ERR! enoent ENOENT: no such file or directory, open '/srv/www/wordpress-develop/public_html/node_modules/jquery-color/node_modules/nyc/node_modules/yargs/node_modules/cliui/package.json.3362228139'
npm ERR! enoent This is related to npm not being able to find a file.

[omarreiss]

Fixed jQuery color. Added .nvmrc and .npmrc files with same defaults as Gutenberg.

[ocean90]

js-organization-6.patch​ includes a few unrelated changes/reverts like for wp-admin/admin-header.php or src/wp-admin/css/dashboard.css.

[omarreiss]

Thanks for catching that @ocean90! Don't know how that happened, but should be fixed now.

[jorbin]

In 42887:

Bump grunt-patch-wordpress to 1.0.0

Adds the ability to do file mapping (see #43055 ), reformats all of the code to bring it inline with the current WordPress coding standards, and fixes a small number of bugs.

Props pento, netweb, boblinthorst, ireneyoast, omarreiss, herregroen, jorbin
Fixes #43646

[adamsilverstein]

Wild idea: can we consider building into src instead of into build? That might mean moving all CSS files out of src as well, and building them back into place. Wondering if we can eliminate the build folder entirely, this would let developers continue running their existing local setups without any directory changes, the only new requirement would be running the build step to start and watch for css/js development.

Its a bit odd that with this change as it stands we wind up with three locations for files - the root where source files live (JavaScript), src where some source files live (php, css) and build where everything gets copied to from src. Am I missing something, is there some other reason we need to have separate src and build folders? If not, lets consider make a complete shift and avoiding the double folder build process.

[netweb]

Replying to adamsilverstein:

Wild idea: can we consider building into src instead of into build? That might mean moving all CSS files out of src as well, and building them back into place. Wondering if we can eliminate the build folder entirely, this would let developers continue running their existing local setups without any directory changes, the only new requirement would be running the build step to start and watch for css/js development.

Its a bit odd that with this change as it stands we wind up with three locations for files - the root where source files live (JavaScript), src where some source files live (php, css) and build where everything gets copied to from src. Am I missing something, is there some other reason we need to have separate src and build folders? If not, lets consider make a complete shift and avoiding the double folder build process.

See option 3 in comment:3, I think there more in-depth discussion in Slack at the time though I'd have to double check

[adamsilverstein]

Replying to netweb:

See option 3 in comment:3, I think there more in-depth discussion in Slack at the time though I'd have to double check

I'm suggesting something more radical: eliminating the build folder, moving all js and css files outside src.

[pento]

Here's a brief outline of what I'm currently thinking for this:

• There are a handful of bugs that I'm still running into, @omarreiss has the list, and is working through them.
• We still need to maintain the build folder, as the place where the built version of WordPress lands. The build process also modifies PHP files, so we wouldn't be able to use the src folder as the place where wordpress.zip is created from.
• I'm okay with moving CSS around, if there's an argument for it to be moved. Moving the JavaScript makes sense, particularly with our JS development practices moving to a modern, modular style. I'm not sure that the same argument applies to CSS, though. Either way, I think there needs to be a larger discussion on that, adding it onto this ticket will just hold it up longer. @omarreiss has done an amazing job keeping the patch up to date, lets not burden him further. 🙂 This ticket is going to require several follow up tickets, so a discussion of how we manage CSS files can quite easily fit into there.

On the topic of follow ups! Taken by itself, this change makes developing for WordPress harder. Moving to a compiled methodology for all contributions is a big shift for WordPress, and this change doesn't have the polish we need to be presenting to encourage new contributors. So, while I'm fine with this reorganisation landing in trunk, there are several other changes that I think will be necessary.

• Better, opinionated dev environment documentation. We currently offer a variety of options, but don't really give people a "if you're not sure, do this" document. Rather than encouraging contributors to choose a tool (particular non-technical contributors who have no way to evaluate those tools), we should be doing it for them. Decisions, not options. 😉 This is particular important, as grunt watch has awful performance in any environment that uses a VM and shared folders. Choosing the wrong development environment can give a terrible experience.
• Automated development environment setup scripts. The ​Gutenberg setup script walks the user through setting up their development environment. It would obviously need to be written to work on Windows, too, before it was a valid option for Core. That will be fun. 🙂
• Modernising build tools. We currently use a little bit of webpack, and a whole lot of Grunt. This reorg adds a little more webpack usage, and Gutenberg will add a whole lot more. I believe we can standardise everything on webpack, reducing the number of tools we rely on.

So, while trunk will be a little harder to work with after this change lands, the aim is to rapidly iterate on the experience, so we ultimately end up with a simpler, friendlier, more modern development environment that welcomes a wider range of contributors.

[adamsilverstein]

@omarreiss has done an amazing job keeping the patch up to date, lets not burden him further.

Absolutely, lets move forward with this! I mentioned the CSS idea this late on the ticket because thats when I thought of it and certainly don't want to block this work! Lets discuss further in chats or another ticket.

[TJNowell]

This is particular important, as grunt watch has awful performance in any environment that uses a VM and shared folders.

Not if you run it on the host machine. Part of the opposition of VVV to auto running grunt watch is that not every user is interested in developing core, yet an instance is installed out of the box. Running grunt watch in the provisioner would add overhead to every VVV users installs unless they disabled the core dev sites and imo that's an unreasonable ask.

Automated development environment setup scripts. The Gutenberg setup script walks the user through setting up their development environment. It would obviously need to be written to work on Windows, too, before it was a valid option for Core.

I began work on a VVV tool to do this automatically once Vagrant/VirtualBox were installed:

​https://github.com/tomjn/vvv-cd-installer

the goal being that you could copy it off a USB drive and run it, and it would step through every stage of setting up an environment: ​https://github.com/tomjn/vvv-cd-installer

But I see no reason the docker call in the GB script couldn't be a git clone ... cd vvv.. vagrant up --provision

As an aside, if we're moving more to webpack, should we not make use of npm scripts so that we maintain the same commands regardless of the tools behind the scenes? npm run watch npm run build etc

[pento]

I commented a little on ​VVV repo, but to reply to your comments here, too:

Replying to TJNowell:

Not if you run it on the host machine. Part of the opposition of VVV to auto running grunt watch is that not every user is interested in developing core, yet an instance is installed out of the box. Running grunt watch in the provisioner would add overhead to every VVV users installs unless they disabled the core dev sites and imo that's an unreasonable ask.

VVV is a wonderful general purpose tool, if Core's development environment were to use something else at the base, that isn't a slight on VVV: it's simply a case of Core being able to create a tool that's focussed on it's use cases.

With the watch command as the example, I absolutely wouldn't ask you or anyone on the VVV team to impose that overhead on the wider VVV user base.

the goal being that you could copy it off a USB drive and run it, and it would step through every stage of setting up an environment: ​https://github.com/tomjn/vvv-cd-installer

That tool is pretty cool, and certainly something we'd want to make available to all WordPress contributors. A nice GUI to walk through the setup process (both offline at contributor days, as well as online) is a million times better than running a bunch of shell commands.

As an aside, if we're moving more to webpack, should we not make use of npm scripts so that we maintain the same commands regardless of the tools behind the scenes? npm run watch npm run build etc

Yah, we'd definitely move everything to npm scripts.

[omarreiss]

Created separate tickets that could be iterated on after this is done:

• https://core.trac.wordpress.org/ticket/43711 is a ticket about building a better development setup for WordPress core. Any Docker vs Vagrant discussions can go there.
• https://core.trac.wordpress.org/ticket/43731 is a ticket about moving most of the Grunt stuff into the Webpack configuration.
• https://core.trac.wordpress.org/ticket/43732 is a ticket about the wording on the notice which we serve when devs try to run WordPress from src

Will upload a new patch asap which takes care of all remaining issues on my list:

• some failing phpunit tests (this is done)
• node / npm engines in package.json which I'll sync with Gutenberg's
• check if I haven't omitted any unminified scripts from the build which were previously there.

[pento]

node / npm engines in package.json which I'll sync with Gutenberg's

I think we'll need to leave them as is for now: the build server (that commits the built version to core.svn) uses that node version, but I think it needs Systems to upgrade the build server before we can change it. 🙂

[netweb]

Replying to pento:

node / npm engines in package.json which I'll sync with Gutenberg's

I think we'll need to leave them as is for now: the build server (that commits the built version to core.svn) uses that node version, but I think it needs Systems to upgrade the build server before we can change it. 🙂

This is correct, this needs to remain as is, for now, Systems can upgrade the version if and when we need it upgraded though it must remain an exact version, currently this is Node.js 8.9.3

p.s. nvm is not an available option either at the moment, it may be an option once the build server has been upgraded.

[omarreiss]

Ok, left the engines untouched. Fixed the PHP unit tests and an issue where I forgot to commit jQuery.color back. Also rebased on master again, which was fun...

All tests (qunit and phpunit) now also need to load code from build. I think this is warranted because most tests are in fact integration tests and thus assume to be run in a fully functional system. As we modularize and isolate code in the future, we can consider creating true unit tests which include code straight from source. Maybe I should create a ticket for context.

If you try to run phpunit with src as ABSPATH, the unit tests exit with instructions on how to fix so there should be a fairly straightforward path forward.

I think this iteration is as good as complete. Happy we have split out some next steps in the other tickets!

[omarreiss]

I figured a summary of all changes would be helpful at this point:

• Introduces a build step to WordPress core development. WordPress can no longer be run from src.
• Reorganizes the folder structure for the JavaScript in src to the following:

  src/js				| All the JavaScript source.
  ├── _enqueues			| Any script that ends up being enqueued.
  │   ├── admin 			| Procedural scripts ran in the admin.
  │   ├── deprecated		| All deprecated scripts.
  │   ├── lib			| All standalone lib scripts.
  │   ├── vendor			| All 3rd party deps that can't be managed with NPM.
  │   └── wp			| All scripts that assign something to the wp namespace.
  │       ├── customize		| Anything under wp.customize.
  │       ├── editor		| Anything under wp.editor.
  │       ├── media		| Anything under wp.media.
  │       ├── utils		| Anything under wp.utils.
  │       └── widgets		| jQuery widgets on the wp namespace.
  └── media			| The media library.
  

• Cleans the filenames in src from namespace references because the folder structure now makes that evident.
• Dependencies that can be installed with NPM have been removed from src and are built straight from node_modules. This will make it much easier to manage and update them.
• Dependencies that still need to be manually included in WordPress have been grouped in src/js/_enqueues/vendor. A README was included listing the sources of all of those dependencies. This will make it easier to manage those as well.
• Backwards compatibility is ensured by making sure the files are being built back to the legacy folder/file scheme in production. A future consideration could be made to also reorganize the files in the build.
• Developers who try to run from src will land on a screen in the browser which explains to them which steps they need to take. The wording of that screen can still be improved. There is a separate ticket for that here: https://core.trac.wordpress.org/ticket/43732#ticket
• Tests also need to load code from the build directory. This is because most of the tests in WordPress core are in fact integration tests. The code tested is not isolated and therefore we need to load the entire system in order to test functionality. As we get more modular and isolated code in the future, we should also have a way to run actual unit tests from src.
• The qunit tests have simply been configured to run from build. The phpunit rely on local config so we added a check to make sure the built code is loaded. If this is not the case, we throw an error which explains to the user which config they need to change.
• Grunt watch has been heavily optimized to ensure fast rebuild times. We managed to get rebuilds to be around 1 second by making sure to only rebuild what has been touched. The initial build will of course still be slow.
• Grunt patch was updated to support filemappings. This makes it possible to apply patches to files that were moved.
• Drafts for documentation exist in ​https://drive.google.com/open?id=1FDD8QQ5TmydZ62nMzPb8pmiVpUSCnhsv. There are 4 documents in total with the following topics:
  • JavaScript code organization and development practices
  • JavaScript modules and Webpack
  • Managing dependencies
  • Setting up your local development environment

To further improve the development workflow, two more tickets were created:

• Let's create a standard development setup for WordPress core. This ticket aims to simplify the development setup for WordPress to be as close as possible to a "download and boot" scenario.
• Use Webpack + NPM scripts to build all the things. This ticket aims to unify the building process and reduce the complexity and number of different build tools.
• A third ticket could still be created to also reorganize the CSS in a similar fashion as we've now done for the JavaScript.

[nbachiyski]

One forward-facing requirement I’d add is to not depend on grunt, npm, or webpack in the CLI tools or configuration of the new build system – the same way WordPress hasn't depended on any framework, even Gutenberg has abstracted away the React specifics. The only two user-facing dependancies we should be OK with are php and node, IMHO, even if we use the latest-and-shiny under the hood (well, grunt hasn't be new or shiny for a long time and will probably go away, which is a great example of why we should have our own façade in front of the JS hotnesses).

[omarreiss]

Rebased the patch on the latest master. My branch also lives on Github on the Yoast mirror: ​https://github.com/Yoast/wordpress-develop-mirror/tree/rebased-js-src-move/. @pento I gave you write access to that repo so things will potentially be easier for you.

[omarreiss]

Run this script before applying the patch to ensure clean svn moves.

[omarreiss]

Added a script which creates all the directories under src/js with svn mkdir command, then moves all files there with svn mv commands that need to be moved and removes all dependencies which are no longer needed with svn rm commands. I've created a new patch that should apply all additions and modifications. This should make committing this as easy as possible. The patch is diffed on master. I don't expect this to be a problem, but it might try to delete or move files that are already moved by the script. If this turns out to be the case, I could now easily diff it in a way that takes this into account.

[pento]

In 43309:

Once upon a midnight dreary, while I coded, weak and weary,
In many a strange and curious file of forgotten lore—
While I pondered, blaming Nacin, my notifications suddenly awakened,
As of someone quietly DMing;—DMing me, I can’t ignore.
“’Tis some contributor,” I muttered, “DMing me an idea or four—

Only this and nothing more.”

Ah, distinctly I remember, at WordCamp US, last December;
A mad proposal nearly laid me—down out cold—upon the floor.
Curious, I listened closely;—to a plan I agreed with, mostly—
A way to make our JavaScript—JavaScript which was a chore—
Maintainable, extendable, for the future, is what I saw.

Guten-ready for evermore.

Open here I switch to Slack, when, with many a patch and hack,
In there stepped Omar, a JavaScript developer hardcore;
Pronouncing all the changes fit; ready now to be commit;
“There’s nothing else for us to do,” DMing me, “It’s done!” he swore—
“No longer random guessing at which file need next be explored—

Let’s move on, we’re all aboard.”

Moved all together, grouped and managed, in folders all is packaged,
The code had all been cleaned and tidied, important parts moved to the fore,
“Though this change be useful here,” I said, “it is too large, I fear,
We couldn’t manage such a patch, we’ve done nothing like this before—
Tell me where doth go this change, change to make our codebase soar!”

Quoth Omar, “In WordPress Core.”

Props omarreis for shepherding this significant change.
Props adamsilverstein, aduth, atimmer, dingo_bastard, frank-klein, gziolo, herregroen, jaswrks, jeremyfelt, jipmoors, jorbin, netweb, ocean90, pento, tjnowell, and youknowriad for testing, feedback, discussion, encouragement, commiserations, etc.
I make no apologies for this commit message.
Fixes #43055.

[pento]

In 43310:

Following [43309], I need to change one little line,
And delete some left over cruft, only then shall we prevail.
There are some things easily missed, when using Git, which does not persist—
Empty directories, though that didn't derail—
Our Travis-based tests, which now must prevail.

Quoth Travis CI, “Build did fail.”

See #43055.

[danielbachhuber]

Breaks a ton of plugin / theme unit tests. We should only force build/ for core tests.

[johnbillion]

In 43311:

Build/Test Tools: Roses are red, this fixes stuff.

Update the test infrastructure so that third party plugins, themes, and projects that use the core testing framework continue to operate from the src directory and do not require a build step.

Props mboynes, danielbachhuber, schlessera

See #43055

[desrosj]

Corresponding dev notes:

• ​https://make.wordpress.org/core/2018/05/16/preparing-wordpress-for-a-javascript-future-part-1-build-step-and-folder-reorganization/
• ​https://make.wordpress.org/core/2018/10/19/5-0-js-package-inclusion-update/