Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#46951 closed defect (bug) (fixed)

Editor: Backport packages containing bug fixes from Gutenberg 5.5

Reported by: aduth's profile aduth Owned by: aduth's profile aduth
Milestone: 5.2 Priority: normal
Severity: normal Version: 5.2
Component: Editor Keywords: has-patch commit
Focuses: Cc:

Description

Updated packages:

  • @wordpress/block-editor@2.0.0
  • @wordpress/block-library@2.4.3
  • @wordpress/blocks@6.2.3
  • @wordpress/components@7.3.0
  • @wordpress/dom@2.2.3
  • @wordpress/e2e-tests@1.1.3
  • @wordpress/edit-post@3.3.3
  • @wordpress/editor@9.2.3
  • @wordpress/format-library@1.4.3
  • @wordpress/list-reusable-blocks@1.3.3
  • @wordpress/nux@3.2.3

Process, referencing prior tickets #46801 (Gutenberg 5.4) and #46584 (Gutenberg 5.3):

  1. To capture updates in package.json: Update packages to latest using ncu '/^@wordpress\//' -a -u (ncu)
  2. To capture updates in package-lock.json: npm install
  3. To capture updates in script-loader.php: Manually cross-reference updated packages to assigned version
  4. No block PHP files to update (reference pull request with no `*.php` changes)
  5. No dependencies changes (reference pull request with no `package.json` changes)

Attachments (3)

46951.patch (23.1 KB) - added by aduth 6 years ago.
46951.2.patch (18.2 KB) - added by netweb 6 years ago.
46951.3.patch (18.2 KB) - added by aduth 6 years ago.

Download all attachments as: .zip

Change History (20)

@aduth
6 years ago

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


6 years ago

#2 in reply to: ↑ description ; follow-up: @aduth
6 years ago

Replying to aduth:

  1. To capture updates in package.json: Update packages to latest using ncu '/^@wordpress\//' -a -u (ncu)

Minor correction: The tool is npm-check-updates, if useful for future reference. The command it exposes is ncu.

#3 @aduth
6 years ago

Note: There were 11 published packages, but @wordpress/e2e-tests is not included in core, so there are only 10 package updates reflected in this patch.

#4 in reply to: ↑ 2 @netweb
6 years ago

Replying to aduth:

Replying to aduth:

  1. To capture updates in package.json: Update packages to latest using ncu '/^@wordpress\//' -a -u (ncu)

Minor correction: The tool is npm-check-updates, if useful for future reference. The command it exposes is ncu.

Maybe we should add this to @wordpress/scripts in some form 🤔

#5 follow-up: @netweb
6 years ago

  • Keywords commit added

Patch 46951.patch looks good to me

Aside: Those https<>http changes continue to occur:

  • Version

     
    1541  "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz",
     1541 "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz",
Last edited 6 years ago by netweb (previous) (diff)

@netweb
6 years ago

#6 in reply to: ↑ 5 @netweb
6 years ago

Replying to netweb:

Aside: Those https<>http changes continue to occur:

  • Version

     
    1541  "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz",
     1541 "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz",

Actually, I went and researched this issue a little further, the main discussion appears to be in this npm community thread, the thread is now closed, it also appears to be closed without a resolution either, a workaround solution though is posted here:

npm install --global npm@6.9.0
rm -rf ./node_modules
npm cache clean --force
npm install --prefer-online

After performing the above using the changes in 46951.patch except for the package-lock.json file results in the patch 46951.2.patch which does not include the https to http changes. I'm not sure if future npm install's should include the --prefer-onliune flag or not, time will tell I guess but at least there is a sane workaround for now.

#7 @aduth
6 years ago

Nice sleuthing @netweb . The revised patch looks good, and I agree with an effort to avoid protocol changes (and in keeping protocol to HTTPS).

There's not much documentation about the mentioned --prefer-online flag, but of what I could find, it's not immediately obvious why this would have an effect (and in fact, why it couldn't have the inverse effect of introducing changes than to avoid them). Perhaps the root of the issue stems from how the protocol is referenced in the offline cache.

A new --prefer-online option that will force npm to revalidate cached data (with 304 checks), ignoring any staleness checks, and refreshing the cache with revalidated, fresh data.

https://blog.npmjs.org/post/161081169345/v500

But as far as workarounds go, if it achieves the desired effect, it sounds like it may be worth incorporating into the workflow.

#8 @iseulde
6 years ago

  • Keywords commit removed

@aduth So we're missing a fix here, correct? Removing commit tag.

#9 follow-up: @aduth
6 years ago

There was a bugfix which was missed in the original backport / publish. A follow-up pull request is proposed to include it:

https://github.com/WordPress/gutenberg/pull/15020

There was mention today of another high-priority bug which would be good to have fixed for today's WordPress 5.2 RC:

https://github.com/WordPress/gutenberg/issues/14817

I'm supposing it would be fine if the latest patch of this ticket is committed as-is, as I could create a separate follow-up ticket with the publish which would result from this additional set of backported bug fixes. That said, I think it may also be reasonable to say this existing ticket could be held from commit momentarily to allow those changes to land, and then update the patch accordingly?

Mentioned in Slack: https://wordpress.slack.com/archives/C02QB2JS7/p1555510140245800

#10 @aduth
6 years ago

Out of the aforementioned pull request with additional backporting, a subsequent packages publish was performed. The following packages were updated:

  • @wordpress/block-editor@2.0.1
  • @wordpress/block-library@2.4.4
  • @wordpress/blocks@6.2.4
  • @wordpress/components@7.3.1
  • @wordpress/dom@2.2.4
  • @wordpress/e2e-tests@1.1.4
  • @wordpress/edit-post@3.3.4
  • @wordpress/editor@9.2.4
  • @wordpress/format-library@1.4.4
  • @wordpress/list-reusable-blocks@1.3.4
  • @wordpress/nux@3.2.4

From the patch, I observe that @wordpress/dom package has updated in package.json by _three_ versions (rather than the expected _two_). In investigating this further, it appears this may have been missed in a previous ticket, as the current state of trunk is such that there's a mismatch between package.json and script-loader.php:

https://github.com/WordPress/wordpress-develop/blob/774dcea/package.json#L72
https://github.com/WordPress/wordpress-develop/blob/774dcea/src/wp-includes/script-loader.php#L242

(tl;dr It's expected and in-fact a correction for a previously overlooked issue)

After performing the above using the changes in 46951.patch​ except for the package-lock.json file results in the patch 46951.2.patch​ which does not include the https to http changes. I'm not sure if future npm install's should include the --prefer-onliune flag or not, time will tell I guess but at least there is a sane workaround for now.

While I was eventually successful here, one note is that local changes to package-lock.json should be reset for the corrected updates to be applied.

@aduth
6 years ago

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


6 years ago

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


6 years ago

#13 @desrosj
6 years ago

  • Milestone changed from Awaiting Review to 5.2

#14 in reply to: ↑ 9 ; follow-up: @netweb
6 years ago

Replying to aduth:

There was a bugfix which was missed in the original backport / publish. A follow-up pull request is proposed to include it:

https://github.com/WordPress/gutenberg/pull/15020

There was mention today of another high-priority bug which would be good to have fixed for today's WordPress 5.2 RC:

https://github.com/WordPress/gutenberg/issues/14817

Now that RC1 has been pushed back a week to April 24 waiting to include those here in this ticket should be fine

Patch 46951.3.patch looks good, but lets wait for the additional changes upstream

#15 in reply to: ↑ 14 @aduth
6 years ago

Replying to netweb:

Now that RC1 has been pushed back a week to April 24 waiting to include those here in this ticket should be fine

Patch 46951.3.patch looks good, but lets wait for the additional changes upstream

I realize now it wasn't clear in my previous comment, but the second backporting pull request and subsequent publish (and thus the latest patch) did include the additional bug fix that had been raised in the meeting. There are no other pending fixes for consideration that I'm aware of.

#16 @netweb
6 years ago

  • Keywords commit added


#17 @aduth
6 years ago

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

In 45255:

Editor: Update packages to apply bugfixes from Gutenberg 5.5.

  • @wordpress/block-editor@2.0.1
  • @wordpress/block-library@2.4.4
  • @wordpress/blocks@6.2.4
  • @wordpress/components@7.3.1
  • @wordpress/dom@2.2.4
  • @wordpress/e2e-tests@1.1.4
  • @wordpress/edit-post@3.3.4
  • @wordpress/editor@9.2.4
  • @wordpress/format-library@1.4.4
  • @wordpress/list-reusable-blocks@1.3.4
  • @wordpress/nux@3.2.4

Props iseulde, kjellr, aduth, get_dave, talldanwp, jorgefilipecosta, afercia, nosolosw, jasmussen, netweb .

See https://github.com/WordPress/gutenberg/pull/14987 .
See https://github.com/WordPress/gutenberg/pull/15020 .
Fixes #46951 .

Note: See TracTickets for help on using tickets.