WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 8 months ago

Last modified 5 months 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 13 months ago.
Proposed fix
38292.2.patch (1012 bytes) - added by lukasbesch 13 months ago.
Added documentation and exclude node_modules and CVS by default
38292.3.diff (1.0 KB) - added by rachelbaker 8 months ago.
Added strict check & since tag and adjusted spacing.

Download all attachments as: .zip

Change History (18)

@lukasbesch
13 months ago

Proposed fix

#1 @dd32
13 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
13 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
13 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
13 months ago

Added documentation and exclude node_modules and CVS by default

#4 @ryelle
12 months ago

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

#5 @norcross
8 months ago

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

#6 @Faison
8 months 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
8 months ago

  • Milestone changed from Awaiting Review to 4.7.4

#8 @rachelbaker
8 months 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
8 months 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.


8 months ago

@rachelbaker
8 months ago

Added strict check & since tag and adjusted spacing.

#11 @rachelbaker
8 months 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
8 months 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
8 months 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
8 months 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.

#15 @alexvorn2
5 months ago

[deleted]

Last edited 5 months ago by alexvorn2 (previous) (diff)
Note: See TracTickets for help on using tickets.