WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 3 months ago

#26615 closed enhancement (fixed)

Validate compiled and compressed JS using grunt

Reported by: jorbin Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

In the past we've occasionally run into issues with uglify or other minifyers. Grunt JS-Validate parses JS and thus alerts us to these occasional errors. This does add ~1 second to grunt build, but that's not so bad considering we are validating 276 javascript files.

Attachments (2)

validate.diff (1.4 KB) - added by jorbin 4 months ago.
26615-exclude-wp-content.patch (392 bytes) - added by TobiasBg 3 months ago.
Don't jsvalidate files in wp-content

Download all attachments as: .zip

Change History (8)

jorbin4 months ago

comment:1 wonderboymusic3 months ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 26977:

Add jsvalidate as a dependency to the build. To get the new module, run npm install in the root of the project.

Fixes #26615.
Props jorbin.

comment:2 TobiasBg3 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The src checks too many files here. BUILD_DIR + '/**/*.js' includes JS files of plugins (or other stuff in wp-content).
Not only does this cause extra work during a grunt build, but it can also raise errors where .js files are used for something else (e.g. not holding JS code, but JSON data).

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

comment:3 TobiasBg3 months ago

Besides what I mentioned above, this also tries to validate JS files in npm modules of plugins in wp-content (e.g. if they also use grunt) -- which results in errors like

>> Line 2: Unexpected token :
Validating build/wp-content/plugins/<my_plugin_slug>/node_modules/grunt-contrib-csslint/node_modules/csslint/cli.js  

Thus, we should exclude wp-content here (for which I uploaded a patch). (Drawback: Core themes would aso be exlcuded.)
Other options could be to just jsvalidate .min.js files (which is what this ticket was intended for, I think), although that would also check .min.js files in plugins.
An even better solution might be to check just the files that were created by uglify. My tries with <%= uglify.core.dest %> failed however.

TobiasBg3 months ago

Don't jsvalidate files in wp-content

comment:4 TobiasBg3 months ago

  • Type changed from defect (bug) to enhancement

Just saw that this should go to the 3.9 Milestone, too. @SergeyBiryukov ? :-)

comment:5 ocean903 months ago

  • Milestone changed from Awaiting Review to 3.9

comment:6 wonderboymusic3 months ago

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

In 27054:

Ignore .js files in wp-content when running jsvalidate during grunt build. See [26977].

Props TobiasBg.
Fixes #26615.

Note: See TracTickets for help on using tickets.