Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#38292 closed enhancement (fixed)

Introduce exclusion for WP_Theme::scandir()

Reported by: lukasbesch's profile lukasbesch Owned by: rachelbaker's profile 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 years ago.
Proposed fix
38292.2.patch (1012 bytes) - added by lukasbesch 7 years ago.
Added documentation and exclude node_modules and CVS by default
38292.3.diff (1.0 KB) - added by rachelbaker 7 years ago.
Added strict check & since tag and adjusted spacing.

Download all attachments as: .zip

Change History (18)

@lukasbesch
7 years ago

Proposed fix

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

Added documentation and exclude node_modules and CVS by default

#4 @ryelle
7 years ago

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

#5 @norcross
7 years ago

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

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

  • Milestone changed from Awaiting Review to 4.7.4

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


7 years ago

@rachelbaker
7 years ago

Added strict check & since tag and adjusted spacing.

#11 @rachelbaker
7 years 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
7 years 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
7 years 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
7 years 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
7 years ago

[deleted]

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