Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_ created an issue. See original summary.

nod_’s picture

Status: Active » Needs review
FileSize
5.31 KB

and here it is, was pretty quick thanks to #3219088: Use package.json to manage third party JS libraries

longwave’s picture

Status: Needs review » Reviewed & tested by the community

+1 to the new script, I get exactly the same patch with:

$ yarn upgrade js-cookie@^3.0.0
$ yarn vendor-update
catch’s picture

Status: Reviewed & tested by the community » Needs work

The build successful is deceiving, there's a test failure.

https://dispatcher.drupalci.org/job/drupal_patches/93149/console

13:04:19 TEST FAILURE:  1 assertions failed, 1229 passed (3m 21s)
13:04:19 
13:04:19  ✖ jsCookieTest
13:04:19  – Test jquery.cookie Shim Decode (77ms)
13:04:19    Failed [equal]: ($.cookie returns decoded cookie value) - expected " red panda" but got: "null" 
sagarchauhan’s picture

Status: Needs work » Needs review
FileSize
6.16 KB

Updated js-cookie package to 3.0.1 with commands available in #3. The new version of package handles cookie key in a different way for which earlier patch was failing and hence a fix was added in jquery cookie shim (jquery.cookie.shim.es6.js).
As package version is difference, therefore uploading a new patch altogether.

alexpott’s picture

We're responsible for the encoding so there's no need to do the decoding We can stop doing the encoding.

catch’s picture

Title: Update to js-cookie 3.0.0 » Update to js-cookie 3.0.1
Issue tags: +9.3.0 release notes
longwave’s picture

Status: Needs review » Needs work

So we have a test for this case, that fails on straight upgrade but is fixed by removing a workaround we added in the first place. This looks like the right thing to do so this is nearly RTBC.

Remaining nit: there are two references to 3.0.0-rc.0 in @see tags:

   * @see https://github.com/js-cookie/js-cookie/blob/v3.0.0-rc.0/README.md

Let's update these to the 3.0.1 readme.

alexpott’s picture

Status: Needs work » Needs review
FileSize
930 bytes
7.2 KB

Great spot @longwave. Fixed.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks. RTBC :)

  • catch committed e35c56b on 9.3.x
    Issue #3225811 by alexpott, nod_, sagarchauhan, longwave: Update to js-...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed e35c56b and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

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