Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 12 months ago

#26615 closed enhancement (fixed)

Validate compiled and compressed JS using grunt

Reported by: jorbin's profile jorbin Owned by: wonderboymusic's profile 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 11 years ago.
26615-exclude-wp-content.patch (392 bytes) - added by TobiasBg 11 years ago.
Don't jsvalidate files in wp-content

Download all attachments as: .zip

Change History (9)

@jorbin
11 years ago

#1 @wonderboymusic
11 years 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.

#2 @TobiasBg
11 years 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 11 years ago by TobiasBg (previous) (diff)

#3 @TobiasBg
11 years 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.

@TobiasBg
11 years ago

Don't jsvalidate files in wp-content

#4 @TobiasBg
11 years ago

  • Type changed from defect (bug) to enhancement

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

#5 @ocean90
11 years ago

  • Milestone changed from Awaiting Review to 3.9

#6 @wonderboymusic
10 years 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.

#7 @kadamwhite
12 months ago

In 56247:

Build Tools: Remove outmoded jsvalidate task from Gruntfile.

grunt-jsvalidate was added to catch situations where minification introduced parsing errors in Internet Explorer.
IE is no longer supported by WordPress, and the grunt task itself has not been maintained for a decade.
Removing this task simplifies the build and avoids false-positive warnings where the outdated tool flags modern JS syntax incorrectly.

Props westonruter, joemcgill, jorbin, youknowriad, desrosj, swissspidy.
Fixes #58645. See #26615.

Note: See TracTickets for help on using tickets.