WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 5 weeks ago

#38292 closed enhancement (fixed)

Introduce exclusion for WP_Theme::scandir()

Reported by: lukasbesch Owned by: rachelbaker
Milestone: 4.7.4 Priority: normal
Severity: normal Version: 4.6.1
Component: Themes Keywords: has-patch fixed-major
Focuses: administration, performance Cc:

Description

I am using NPM to manage my theme's dependencies during development and ran into the following issue where accessing wp-admin/edit.php?post_type=page returned an empty response. Tracing it down I found out that WordPress is looking for possible page templates in the theme directory. Scanning the node_modules folder is probably too much a server can handle.

To solve this problem I suggest to introduce an exclusion list so theme developers can add directories (and files) that will be ignored when searching for post templates. I am not aware of any disadvantages of this solution and don't exactly know where the function is used elsewhere.

Attachments (3)

38292.patch (607 bytes) - added by lukasbesch 7 months ago.
Proposed fix
38292.2.patch (1012 bytes) - added by lukasbesch 7 months ago.
Added documentation and exclude node_modules and CVS by default
38292.3.diff (1.0 KB) - added by rachelbaker 6 weeks ago.
Added strict check & since tag and adjusted spacing.

Download all attachments as: .zip

Change History (17)

@lukasbesch
7 months ago

Proposed fix

#1 @dd32
7 months ago

given the line before it is if ( '.' == $result[0] ) continue; which is mostly used to skip .svn folders, and there's a CVS check within the next block, it's probably worth just hard-coding node_modules into the code here to skip for everyone (given how common it is to use a node-based development method today).

To make it more readable we can define those skip-directories as an array that's filtered if it makes it more readable.

#2 @lukasbesch
7 months ago

  • Keywords has-patch needs-testing added

There shouldn't be a reason to scan node_modules, so we should be safe to hardcode. But let's keep it filterable yet so one can add other package managers or libraries to that array.

#3 @swissspidy
7 months ago

@lukasbesch Thanks for your patch! Would you mind adding documentation according to the inline docs standards? See https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/. Also, braces for conditions are a requirement for newly added code as per our coding standards.

@lukasbesch
7 months ago

Added documentation and exclude node_modules and CVS by default

#4 @ryelle
5 months ago

I was also having this problem, and the patch fixes it 👍

#5 @norcross
6 weeks ago

good work @lukasbesch I ran into this issue myself and this patch solved the problem.

#6 @Faison
6 weeks ago

Thanks @lukasbesch , I just learned that this issue is making my local WP REST API requests pretty slow. With your patch I'm saving nearly half a second in my response time :D

#7 @rachelbaker
6 weeks ago

  • Milestone changed from Awaiting Review to 4.7.4

#8 @rachelbaker
6 weeks ago

  • Owner set to rachelbaker
  • Status changed from new to reviewing

Seems like more and more folks are running into this issue as they create REST API powered themes. We should fix this in the next minor release.

#9 @swissspidy
6 weeks ago

The in_array() check in the current patch should be a strict check.

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


6 weeks ago

@rachelbaker
6 weeks ago

Added strict check & since tag and adjusted spacing.

#11 @rachelbaker
6 weeks ago

In 38292.3.diff I made some minor code style adjustments, added the @since tag to the filter documentation, and used a strict in_array() check. @swissspidy can you look this over to make sure I didn't miss anything?

#12 @rachelbaker
6 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 40301:

Themes: Add filter for excluding directories from being scanned for template files.

Exclude 'node_modules' directories from paths searched in WP_Theme::scandir(). Introduces the theme_scandir_exclusions filter to allow sites to exclude any other paths like bower_components or vendor from being searched for template files.

Props lukasbesch, dd32, swisspidy, rachelbaker.
Fixes #38292.

#13 @rachelbaker
6 weeks ago

  • Keywords fixed-major added; needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for porting to the 4.7.4 branch.

#14 @swissspidy
5 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 40326:

Themes: Add filter for excluding directories from being scanned for template files.

Exclude 'node_modules' directories from paths searched in WP_Theme::scandir(). Introduces the theme_scandir_exclusions filter to allow sites to
exclude any other paths like bower_components or vendor from being searched for template files.

Props lukasbesch, dd32, swisspidy, rachelbaker.
Fixes #38292.

Merges [40301] to the 4.7 branch.

Note: See TracTickets for help on using tickets.