WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 3 months ago

#46099 assigned defect (bug)

Ignore theme based node_modules

Reported by: nielslange Owned by: nielslange
Milestone: Future Release Priority: normal
Severity: minor Version: 5.0
Component: Build/Test Tools Keywords: has-patch needs-testing
Focuses: Cc:

Description

Recently, the Twenty Nineteen theme was moved from Github into Trac. Twenty Nineteen is the first theme that is using npm and therefore has a node_modules directory within the theme folder.

When running grunt, grunt build or grunt watch multiple errors occur and the command gets aborted. While this could be bypassed using grunt --force, grunt build --force and grunt watch --force, a better approach would be to ignore the theme based node_module folder.

Steps to reproduce

  1. Navigate to /src/wp-content/themes/twentyninteen
  2. Run npm install
  3. Navigate to /
  4. Run grunt

Expected result

  • No error occurs.

Current result

  • The following error occurs:
Running "jsvalidate:build" (jsvalidate) task
...
...
...
Encountered 292 errors.
Warning: Task "jsvalidate:build" failed. Use --force to continue.
Aborted due to warnings.

Attachments (1)

46099.diffโ€‹ (529 bytes) - added by nielslange 3 months ago.

Download all attachments as: .zip

Change History (5)

#1 @nielslange
3 months ago

  • Severity changed from normal to minor

@allancole & @kjellr: Feel free to test my patch. ๐Ÿ˜‰

PS: While I created this patch to solve the concrete issue I ran into while working on Twenty Nineteen related tickets, I created the patch in a way that it affects all themes that have a node_modules directory.

PPS: The quickest way to test this patch is by running grunt jsvalidate:build directly.

Last edited 3 months ago by nielslange (previous) (diff)

#2 follow-up: @kjellr
3 months ago

  • Component changed from Bundled Theme to Build/Test Tools

Awesome, thank you @nielslange!

This works on my end, but I'd like for someone with some experience in that Gruntfile to take a closer look. Going to reassign this to the "Build/Test Tools" component for some more eyes there.

#3 in reply to: โ†‘ย 2 @nielslange
3 months ago

Youโ€™re very welcome, @kjellr, and thanks for changing the component to Build/Test tools. Thatโ€™s definitely a better component for this change. ๐Ÿ˜‰

#4 @desrosj
3 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Version changed from trunk to 5.0
Note: See TracTickets for help on using tickets.