Make WordPress Core

Opened 5 years ago

Last modified 4 years ago

#49274 new enhancement

Grunt copy:files should ignore node_modules

Reported by: iandunn's profile iandunn Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

I've gotten this error several times when run grunt watch, and several others have too:

Maximum call stack size exceeded

It happens because there's a node_modules folder somewhere in src/, and they typically contain too many files for Grunt to handle.

Configuring copy:files to always ignore node_modules folders seems like it'd remove that friction for people.

Attachments (3)

49274.1.diff (567 bytes) - added by iandunn 5 years ago.
Adds node_module ignore to _watch task.
49274.2.diff (638 bytes) - added by iandunn 5 years ago.
1.diff + ignoring build and build-* folders
49274.3.diff (381 bytes) - added by iandunn 5 years ago.
different approach, ignore all of wp-content

Download all attachments as: .zip

Change History (16)

@iandunn
5 years ago

Adds node_module ignore to _watch task.

#1 @iandunn
5 years ago

49274.1.diff works for me, but could use some more eyes/testing.

  1. Clone several plugins that depend on many NPM packages into src/wp-content/plugins (e.g., gutenberg, yoast, coblocks, etc)
  2. Run npm install in each of their directories
  3. Go up to src and run npm run dev
  4. Change some files in src. If there are enough files inside plugins/**/node_modules, then you'll get this error from the watch task:
Running "_watch" task
Waiting...
Warning: Maximum call stack size exceeded
  1. Cancel the npm run dev task
  2. Apply the patch
  3. Restart npm run dev, and change some files.

This ticket was mentioned in Slack in #core by iandunn. View the logs.


5 years ago

#3 @iandunn
5 years ago

It looks like the build, build-module, build-types, etc folders inside plugins/gutenberg also cause this problem. We could exclude those explicitly, or maybe do that for all plugins? There may be some where it's necessary to watch build folder, though?

This is helpful for finding the folders with the most files:

for i in `find src -type d `; do echo `ls -a $i | wc -l` $i; done | sort -n

@iandunn
5 years ago

1.diff + ignoring build and build-* folders

@iandunn
5 years ago

different approach, ignore all of wp-content

#4 @iandunn
5 years ago

49274.2.diff adds additional ignore entries for build and build-* folders, which fixes the max call stack error with Gutenberg (comment:3). The watch task takes ~25 seconds to run, though, so it's still unusable.

49274.3.diff takes a more radical approach, and ignores all of wp-content. This solves the speed issue, but has the (somewhat) unintended consequence of preventing mu-plugins, plugins, and themes from being copied from src to build.

I often need to test plugins against trunk, especially Gutenberg, so that seems like a reasonable use case. People could create & build those things directly in WP's build folder, but it doesn't seem like build is really a safe location for anything that isn't in src. I often rm -rf build when troubleshooting.

Does anyone have any thoughts?

#5 @iandunn
5 years ago

Part of the problem is that too many files are being watched, but another part might be that sometimes there are two watcher tasks running simultaneously.

If grunt watch and grunt watch:phpunit are both running simultaneously, then that'd probably make the problem much worse.

The Handbook says that npm run grunt watch -- --phpunit --group={testgroup} will run the normal watch tasks and watch:phpunit simultaneously, but that isn't working for me. Apparently I added that note, but don't remember the details. It's also possible some things have changed in the meantime, and the docs are out of date.

This ticket was mentioned in Slack in #core by iandunn. View the logs.


5 years ago

#7 @iandunn
5 years ago

Even without running grunt watch:phpunit, npm run watch still takes 10s per cycle on my system, when using 49274.2.diff.

For 49274.3.diff, one potential solution to the syncing problem might be to symlink build/wp-content to src/wp-content, or at least the mu-plugins/ and plugins/ folders, since Core doesn't have anything in those. I'll test that for awhile and see if I notice any problems.

#8 @iandunn
5 years ago

So far, the only problem I've run into with symlinking wp-content is that these files are sometimes deleted during build and watch tasks:

src/wp-content/index.php
src/wp-content/plugins/hello.php
src/wp-content/plugins/index.php
src/wp-content/themes/*

One potential solution would be to adjust Gruntfile.js to avoid that, but I haven't dug into that, and any unintended consequences it'd have.

For now I'm just symlinking everything inside wp-content except those files. It seems to work fine at first glance, but I'll keep working with it for a few days.

Related: #44492, where symlinking was tried for most of the src/ folder, but that didn't work out because of problems with Windows support.

cc @netweb, @omarreiss, @SergeyBiryukov

#9 @netweb
5 years ago

Related: #43731Use Webpack + NPM scripts to build all the things

It might be a good opportunity explore an alternate file watcher, and move away from the Grunt task and it’s limitations

Adding a native npm watch script and then wrapping a back-compact Grunt task

Facebook’s Watchman comes to mind, Windows is still officially beta, but appears to be well tested.

#10 @iandunn
4 years ago

#50412 is also related, since having two watchers running appears to be part of the problem.

#11 @isabel_brison
4 years ago

Tested and patch 3 is the one that works best against a full Gutenberg clone in the plugins folder.

I often need to test plugins against trunk, especially Gutenberg

How often would it be necessary to watch both Core and Gutenberg files simultaneously though? Would you be working on changes in both codebases at once? Asking also because knowing more about these use cases can help inform future improvements to @wordpress/env :)

My use cases are usually either checking a Gutenberg feature branch I'm working on against the latest trunk or (more rarely) checking a Core feature branch I'm working on against the latest from Gutenberg; in either case I only need to watch for changes in one of those places, so patch 3 would work fine as is.

#12 @iandunn
4 years ago

Thanks for testing!

My most common use case w/ Gutenberg is reproducing a potential bug on a fresh WP install. Testing a PR against WP could be another common one.

IIRC, the reason running both tasks is sometimes needed, is because WP is often served from the build/ dir. So,

  1. Dev changes a file in Gutenberg src/wp-content/plugins/gutenberg/packages
  2. Gutenberg's dev task builds the changes into Gutenberg's build/ folder
  3. Core's watch task copies src/wp-content/plugins to build/wp-content/plugins
  4. Nginx serves Gutenberg from build/wp-content/plugins/gutenberg/build

If Core's task weren't also running, then it seems like refreshing would get the older version of Gutenberg.

#13 @iandunn
4 years ago

One workaround for the symlink issue (comment:7) is using a custom WP_CONTENT_DIR. I like doing that anyway, but any general-purpose solution would probably need to handle the default install as well.

Note: See TracTickets for help on using tickets.