Problem/Motivation

According to https://www.drupal.org/project/drupal/issues/3238507 we have to update terser and terser-webpack-plugin packages to the latest version

Issue fork drupal-3296481

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kostyashupenko created an issue. See original summary.

kostyashupenko opened merge request !2522

kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko at Skilld commented
Assigned: kostyashupenko » Unassigned
Status: Needs work » Needs review
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented
Component: base system » javascript
Status: Needs review » Reviewed & tested by the community

works for me, no changes to built files.

  • xjm committed 02defec on 10.1.x 
    Issue #3296481 by kostyashupenko, nod_: Update terser and terser-webpack...

  • xjm committed dee9b37 on 10.0.x 
    Issue #3296481 by kostyashupenko, nod_: Update terser and terser-webpack...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Version: 10.0.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Needs work

Confirmed that the lock file matches what I get when I do the update myself, and that there are no changes to built assets as a result of the update.

Committed to 10.1.x and 10.0.x. Thanks!

We also need versions of this for 9.5.x, 9.4.x, and 9.3.x.

xjm opened merge request !2592

xjm reopened merge request !2590

xjm reopened merge request !2589

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs work » Needs review

Opened MRs for 9.5.x, 9.4.x, and 9.3.x.

xjm closed merge request !2522

  • xjm committed fc2f8a4 on 10.1.x 
    Revert "Issue #3296481 by kostyashupenko, nod_: Update terser and terser...

  • xjm committed e67c63a on 10.0.x 
    Revert "Issue #3296481 by kostyashupenko, nod_: Update terser and terser...

xjm reopened merge request !2522

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

Actually I discovered that there are changes to the jQuery UI assets that were missing from the patch. These are separate changes from the jQuery UI 1.13.2 update, but are necessary since the update. Basically, the webpack update is changing how they're packaged.

I reverted the previous commit, then reopened the 10.0.x MR and pushed a commit that rebuilds the assets.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented
Status: Needs review » Reviewed & tested by the community
Related issues: +#3301712: Add vendor-update command to the build command for JS assets

ran the commands and ended up with the same patches.

Opened related issue to avoid what happened here.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

We need to rebase all MRs now that #3300040: Update jQuery UI to the latest versions is done.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented

Btw the policy as laid out in #3280275: Set up a formal process for ensuring JavaScript dependencies remain up to date would not have caught this problem

#3301712: Add vendor-update command to the build command for JS assets allows us to fix this without changing the policy or our habits.

xjm opened merge request !2602

xjm closed merge request !2522

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs work » Needs review

Force-pushed updated branches (I just rebuilt them on the branch tip, rather than rebasing, since the assets probably differ.)

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

ran the commands on the 4 branches and got the same patch. All good for me.

  • lauriii committed baf3e89 on 10.1.x 
    Issue #3296481 by xjm, kostyashupenko, nod_: Update terser and terser-...

  • lauriii committed 6886b13 on 10.0.x 
    Issue #3296481 by xjm, kostyashupenko, nod_: Update terser and terser-...

lauriii closed merge request !2602

  • lauriii committed bc2195e on 9.5.x 
    Issue #3296481 by xjm, kostyashupenko, nod_: Update terser and terser-...

lauriii closed merge request !2589

lauriii closed merge request !2590

  • lauriii committed e23b697 on 9.4.x 
    Issue #3296481 by xjm, kostyashupenko, nod_: Update terser and terser-...

lauriii closed merge request !2592

  • lauriii committed 44bdc7a on 9.3.x 
    Issue #3296481 by xjm, kostyashupenko, nod_: Update terser and terser-...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Fixed

Confirmed that I get locally the same changes and all looked good. Committed all MRs to their respective branches.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.