Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53719 closed defect (bug) (fixed)

`grunt clean:css` does not clean the `css/dist` folder

Reported by: desrosj's profile desrosj Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 5.8.1 Priority: highest omg bbq
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

The grunt clean:css command is not currently cleaning the css/dist folder. This can result in unexpected files being present locally when distributed CSS files are removed or renamed since these files are not under version control.

Attachments (1)

53719.diff (428 bytes) - added by desrosj 3 years ago.

Download all attachments as: .zip

Change History (17)

@desrosj
3 years ago

#1 @netweb
3 years ago

  • Keywords commit added

Running grunt clean:css with 53719.diff confirms the files (and path) dist is deleted from:

  • wordpress-develop/build/wp-includes/css

#2 @peterwilsoncc
3 years ago

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

In 51689:

Build: Remove css/dist in grunt clean command.

Modify the grunt clean:css command to include the folder wp-includes/css/dist to ensure legacy files do not remain if the built files are removed/relocated.

Props desrosj, netweb.
Fixes #53719.

#3 @peterwilsoncc
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging to the 5.8 branch.

#4 @desrosj
3 years ago

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

In 51691:

Build: Remove css/dist in grunt clean command.

Modify the grunt clean:css command to include the folder wp-includes/css/dist to ensure legacy files do not remain if the built files are removed/relocated.

Props desrosj, netweb., peterwilsoncc.
Merges [51689] to the 5.8 branch.
Fixes #53719.

#5 follow-up: @peterwilsoncc
3 years ago

  • Keywords has-patch commit fixed-major removed
  • Priority changed from normal to highest omg bbq
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening as this appears to have an order of operations issue.

build:webpack-assets places a bunch of CSS files in css/dist
build:css then runs clean:css which deletes the newly minted files.

Either a patch or revert incoming.

Bumping the priority to blocker as not having CSS seems pretty much of a blocker to me.

Additional props due: @tellyworth @ramonopoly following discussion in Slack.

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


3 years ago

#8 in reply to: ↑ 5 @netweb
3 years ago

Replying to peterwilsoncc:

Reopening as this appears to have an order of operations issue.

build:webpack-assets places a bunch of CSS files in css/dist
build:css then runs clean:css which deletes the newly minted files.

I think so, looking at the Grunt task build order, a quick summary:

After webpack:* tasks are run, the clean:css task is run again

❯ npm run build

> WordPress@5.9.0 build /Users/netweb/Code/wordpress-develop
> grunt build

package.json has not been modified.
Running "build" task

Running "clean:files" (clean) task
>> 3227 paths cleaned.

...

Running "clean:webpack-assets" (clean) task
>> 0 paths cleaned.

Running "webpack:prod" (webpack) task
Hash: 690f241987760f4a831e2fa831a8185fd37e781d093304c4f1ae113230c0
Version: webpack 4.43.0

Running "webpack:dev" (webpack) task
Hash: 727e0b23022f0575688b2fa831a8185fd37e781d77b719b272f77a0b7ef8
Version: webpack 4.43.0

Running "clean:css" (clean) task
>> 135 paths cleaned.

...

Done.

Moving the clean WORKING_DIR + 'wp-includes/css/dist/' from clean:css to clean:webpack-assets might be the fix:

  • Gruntfile.js

    diff --git Gruntfile.js Gruntfile.js
    index d273eac5e6..369079dc1d 100644
    module.exports = function(grunt) { 
    115115                                WORKING_DIR + 'wp-admin/css/*-rtl*.css',
    116116                                WORKING_DIR + 'wp-includes/css/*.min.css',
    117117                                WORKING_DIR + 'wp-includes/css/*-rtl*.css',
    118                                 WORKING_DIR + 'wp-includes/css/dist/',
    119118                                WORKING_DIR + 'wp-admin/css/colors/**/*.css'
    120119                        ],
    121120                        js: [
    module.exports = function(grunt) { 
    124123                        ],
    125124                        'webpack-assets': [
    126125                                WORKING_DIR + 'wp-includes/assets/*',
     126                                WORKING_DIR + 'wp-includes/css/dist/',
    127127                                '!' + WORKING_DIR + 'wp-includes/assets/script-loader-packages.php'
    128128                        ],
    129129                        dynamic: {

This ticket was mentioned in Slack in #meta by netweb. View the logs.


3 years ago

#10 @ramonopoly
3 years ago

I've tested the patch over at https://github.com/WordPress/wordpress-develop/pull/1645

It looks to be the same approach in https://core.trac.wordpress.org/ticket/53719#comment:8

Running npm run build with the patch applied builds the editor css assets under wp-includes/css/dist/

Thank you!

#11 @peterwilsoncc
3 years ago

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

In 51713:

Build: Clean css/dist as part of the webpack build step.

Move the cleaning of the wp-includes/css/dist folder from clean:css to clean:webpack-assets to avoid an order of operations issue in which the files were built shortly before been deleted later in the build process.

Follow up to [51689].

Props netweb, ramonopoly, peterwilsoncc.
Fixes #53719.

#12 @peterwilsoncc
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to for merging [51713] in to the 5.8 branch.

#13 @peterwilsoncc
3 years ago

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

In 51714:

Build: Clean css/dist as part of the webpack build step.

Move the cleaning of the wp-includes/css/dist folder from clean:css to clean:webpack-assets to avoid an order of operations issue in which the files were built shortly before been deleted later in the build process.

Follow up to [51689].

Props netweb, ramonopoly, peterwilsoncc.
Merges [51713] to the 5.8 branch.
Fixes #53719.

This ticket was mentioned in Slack in #core-editor by ramonopoly. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-editor by andrewserong. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.