WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 18 months ago

#29883 new defect (bug)

Split Gruntfile.js into multiple files

Reported by: jorbin Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch dev-feedback
Focuses: javascript Cc:
PR Number:

Description

Our Gruntfile.js is beginning to become unwieldy. In order to make it easy to read and edit in the future, we should divide it up into multiple tasks.

Attachments (1)

grunt.diff (32.9 KB) - added by voldemortensen 5 years ago.

Download all attachments as: .zip

Change History (14)

This ticket was mentioned in IRC in #wordpress-dev by jorbin. View the logs.


5 years ago

#2 @kadamwhite
5 years ago

I'm taking a stab at this at the WCSF contributor day

@voldemortensen
5 years ago

#3 follow-up: @voldemortensen
5 years ago

  • Keywords has-patch added

This patch takes all of the configurations and loads the into a wpGruntfile module that can then be called in Gruntfile.js. This patch considerable reduces the size of Gruntfile.js and breaks each task into it's own file.

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


5 years ago

#5 in reply to: ↑ 3 @GaryJ
5 years ago

Replying to voldemortensen:

This patch takes all of the configurations and loads the into a wpGruntfile module that can then be called in Gruntfile.js. This patch considerable reduces the size of Gruntfile.js and breaks each task into it's own file.

The load-grunt-config package does similar, but also supports JIT loading of the configs required for that particular run i.e. no need to load all configs when only doing one particular task. It also loads the configs directly by filename, so no need to explicitly export each one.

I like the one-file-per-task approach, but I know that Jorbin has previously said he's not.

#6 @swissspidy
5 years ago

+1 for using load-grunt-config. No need to reinvent the wheel :)

#7 @jorbin
5 years ago

I'm not a fan of the one file per task approach for a few reasons:

1) It makes it harder for people to share configurations of tasks
2) It can very quickly become an issue of having too many files. 15 different files is a lot of places to need to look in order to find
3) As we make changes to tasks (such as the switch from cssjanus to rtlcss), we delete files which makes it harder to do blames and find out why things changed.

One more specific thing from this patch is that I don't like the idea of having our grunt configuration as a dependency. When we split this, I think it should go into tools/grunt and should stay apart of our core repository. Build tools are first class and moving them out of the core repository makes them more hidden and harder for people to patch.

#8 @kadamwhite
5 years ago

As noted above, I was working on this at WCSF until @jorbin asked me to stop. I agree with some of his concerns with this patch, and I disagree with others. To wit:

  • I strongly support multiple files. It (subjectively) makes it easier to find the code you are looking for, not harder, when each file has a clearly separated set of functionality.
  • It is possible, possibly even recommended, to fragment the tasks by filename not based on what plugin we're using, but by the type of task being done. So tools/grunt/css-style.js (e.g.) stores whatever cssjanus or rtlcss option we're choosing, and possibly any other CSS lint-type tasks. That way a shift in the tools used is clearly reflected in the history.
  • Putting first-party code into node_modules is an antipattern. We should keep the task definitions inside the appropriate tooling folders, and require them directly. Plugins can be externalized (e.g. grunt-patch-wordpress), but the actual invocations of those plugins do belong alongside the code they touch.

#9 @netweb
4 years ago

#35523 was marked as a duplicate.

#10 @ericlewis
4 years ago

  • Focuses javascript added

Let's move forward with this. The loading of our modules is rather slow, and it sounds like whatever solution we go with here will add a JIT task loader.

@jorbin what did you think of @kadamwhite's notes on separate files? I like the idea of task responsibility filenames. This would preserve history over changes like when autoprefixer was replaced with postcss.

#11 @ericlewis
4 years ago

  • Milestone changed from Awaiting Review to Future Release

#12 @swissspidy
3 years ago

  • Keywords dev-feedback added

Quoting Eric above: @jorbin what did you think of @kadamwhite's notes on separate files?

This ticket was mentioned in Slack in #core-js by netweb. View the logs.


18 months ago

Note: See TracTickets for help on using tickets.