Opened 2 weeks ago
Last modified 3 days ago
#62887 assigned task (blessed)
Editor: Update packages for 6.8 Betas
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Update the packages worked on in the Gutenberg repo for WordPress 6.8.
Change History (19)
This ticket was mentioned in PR #8224 on WordPress/wordpress-develop by @Mamaduka.
2 weeks ago
#1
- Keywords has-patch has-unit-tests added
@Mamaduka commented on PR #8224:
2 weeks ago
#2
I've not been involved in these syncs in the past, but was under the impression that we were attempting to move away from shipping these kinds of features under the experimental… prefix.
@joemcgill, that only applies to JS API. The block.json
is a configuration file that can't have private APIs, hence the __experimental
prefixes. They're and will be backward compatible.
@joemcgill commented on PR #8224:
2 weeks ago
#3
Thanks for clarifying! Do we have any documentation or best practice for why and when to use those prefixes for block.json features? If they are intended to be backward compatible, should we be looking to remove these prefixes?
@Mamaduka commented on PR #8224:
2 weeks ago
#4
These aren't new configuration values; they just got enabled for those blocks. When those features are stable, we usually create iteration issues like #64314 and stabilize the keys.
I don't recall any official docs, but it is probably worth documenting.
@Mamaduka commented on PR #8224:
11 days ago
#5
Updated packages to fix private API errors breaking the Site Editor (https://github.com/WordPress/gutenberg/pull/68964). It should be working correctly now.
Remaining issues:
- I noticed the "Go to the Dashboard" buttons don't work in the Site Editor. It requires changes to block editor settings from #7903. I can cherry-pick them in this PR to unblock merging.
- Some JavaScript CI tests fail due to this warning -
Warning: The build/wp-includes/js/dist/data.js file must not contain a sourceMappingURL. Use --force to continue.
Does anyone have experience fixing it?
## Screeenshot
@Mamaduka commented on PR #8224:
11 days ago
#6
It looks like wp-polyfill
has been reintroduced as a dependency, which causes the test_wp_add_inline_script_before_after_concat_with_core_dependency
unit test to fail. It was initially removed via https://core.trac.wordpress.org/ticket/60962.
@swissspidy, any suggestions on how to approach this?
@swissspidy commented on PR #8224:
11 days ago
#7
Hmm good question, I wonder what caused this.
Last time this topic came up was in https://github.com/WordPress/gutenberg/issues/66552 and https://github.com/WordPress/gutenberg/pull/67230
A quick test here shows that wp-polyfill
is now suddenly a dependency of wp-i18n
, hence this unit test is failing with the new diff.
I thought with https://github.com/WordPress/gutenberg/pull/65292 / https://github.com/WordPress/gutenberg/pull/65582 the wp-polyfill
dependency should only be added when needed.
Tinkering with the Gutenberg package build process I see that the i18n package now tries to load these two polyfills for iterator helpers (which are currently not supported by Safari) in create-i18n.js
:
import "core-js/modules/esnext.iterator.constructor.js"; import "core-js/modules/esnext.iterator.for-each.js";
And that's why ultimately the wp-polyfill
dependency is added.
create-i18n.js
uses new Set()
and Set.forEach()
, so that's why. But those are supported in Safari, so adding the iterator polyfills is incorrect.
I wonder if we should do something like https://github.com/WordPress/gutenberg/pull/67230 but for Iterator
. That would solve this issue. cc @sgomes
Related: it can't hurt to update Babel & core-js to the latest versions in the Gutenberg repo, as this will affect polyfills too.
@sergiomdgomes commented on PR #8224:
11 days ago
#8
The reason why polyfills are being added unnecessarily is due to fundamental limitations in core-js
, the JS polyfill library that we use to make wp-polyfill
, and which is used by Babel as well. In a nutshell, it's monolithic where it comes to objects like Set
, and assumes that if a Set
object is present anywhere, it's going to need all its methods, and therefore all the internal dependencies it uses to polyfill them.
This was likely caused by a core-js
update that added new functionality, perhaps https://github.com/WordPress/gutenberg/pull/67708?
@sergiomdgomes commented on PR #8224:
11 days ago
#9
To follow up on the above, the aforementioned Gutenberg PR updated core-js
to version 3.39, which according to the project changelog added the Iterator
functionality, so we have our smoking gun.
@gziolo: Would it be worth it to replicate the unit test that's failing here (test_wp_add_inline_script_before_after_concat_with_core_dependency
) at the Gutenberg level, so that we catch these situations in the Gutenberg repo, before they make their way to Core?
@Mamaduka commented on PR #8224:
11 days ago
#10
Thanks, @swissspidy, @sgomes.
So, for now, would the proposed resolution be something similar to https://github.com/WordPress/gutenberg/pull/67230?
@swissspidy commented on PR #8224:
11 days ago
#11
In that case I would say so, yes.
Adding a unit test or CI check for this stuff would be nice for sure.
@Mamaduka commented on PR #8224:
10 days ago
#12
Here's the PR to fix this problem - https://github.com/WordPress/gutenberg/pull/69070.
10 days ago
#13
Would it be worth it to replicate the unit test that's failing here (
test_wp_add_inline_script_before_after_concat_with_core_dependency
) at the Gutenberg level, so that we catch these situations in the Gutenberg repo, before they make their way to Core?
Sure, that makes perfect sense. It looks like @Mamaduka added some test coverage in https://github.com/WordPress/gutenberg/pull/69070 that will help to catch similar regressions.
@Mamaduka commented on PR #8224:
9 days ago
#14
All outstanding issues for this PR are resolved now. We're good to commit if we decide to do so.
@joemcgill commented on PR #8224:
8 days ago
#18
Committed (clumsily) to trunk in:
A list of known issues:
Trac ticket: https://core.trac.wordpress.org/ticket/62887