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 | Owned by: | 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):
- To capture updates in
package.json
: Update packages to latest usingncu '/^@wordpress\//' -a -u
(ncu) - To capture updates in
package-lock.json
:npm install
- To capture updates in
script-loader.php
: Manually cross-reference updated packages to assigned version - No block PHP files to update (reference pull request with no `*.php` changes)
- No dependencies changes (reference pull request with no `package.json` changes)
Attachments (3)
Change History (20)
This ticket was mentioned in Slack in #core-editor by aduth. View the logs.
6 years ago
#2
in reply to:
↑ description
;
follow-up:
↓ 4
@
6 years ago
#3
@
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
@
6 years ago
Replying to aduth:
Replying to aduth:
- To capture updates in
package.json
: Update packages to latest usingncu '/^@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:
↓ 6
@
6 years ago
- Keywords commit added
Patch 46951.patch looks good to me
Aside: Those https
<>http
changes continue to occur:
-
Version
1541 "resolved": "http s://registry.npmjs.org/util/-/util-0.10.3.tgz",1541 "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz",
#6
in reply to:
↑ 5
@
6 years ago
Replying to netweb:
Aside: Those
https
<>http
changes continue to occur:
Version
1541 "resolved": "http s://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
@
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
@
6 years ago
- Keywords commit removed
@aduth So we're missing a fix here, correct? Removing commit
tag.
#9
follow-up:
↓ 14
@
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
@
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.
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
#14
in reply to:
↑ 9
;
follow-up:
↓ 15
@
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:
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
@
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.
Replying to aduth:
Minor correction: The tool is npm-check-updates, if useful for future reference. The command it exposes is
ncu
.