Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deps): update wordpress monorepo #41207

Merged
merged 10 commits into from
May 8, 2020
Merged

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Apr 17, 2020

This PR contains the following updates:

Package Type Update Change
@wordpress/a11y dependencies minor 2.8.0 -> 2.9.0
@wordpress/api-fetch dependencies minor 3.12.0 -> 3.14.0
@wordpress/babel-plugin-makepot dependencies minor 3.4.0 -> 3.5.0
@wordpress/base-styles dependencies minor 1.5.0 -> 1.7.0
@wordpress/block-directory dependencies minor 1.6.0 -> 1.8.0
@wordpress/block-editor dependencies minor 3.8.0 -> 3.10.0
@wordpress/block-library dependencies minor 2.15.0 -> 2.17.0
@wordpress/blocks dependencies minor 6.13.0 -> 6.15.0
@wordpress/components dependencies minor 9.3.0 -> 9.5.0
@wordpress/compose dependencies minor 3.12.0 -> 3.14.0
@wordpress/core-data dependencies minor 2.13.0 -> 2.15.0
@wordpress/data dependencies minor 4.15.0 -> 4.17.0
@wordpress/data-controls dependencies minor 1.9.0 -> 1.11.0
@wordpress/dependency-extraction-webpack-plugin dependencies minor 2.4.0 -> 2.6.0
@wordpress/dom dependencies minor 2.8.0 -> 2.9.0
@wordpress/dom-ready dependencies minor 2.8.0 -> 2.9.0
@wordpress/e2e-test-utils dependencies minor 4.4.0 -> 4.6.0
@wordpress/edit-post dependencies minor 3.14.0 -> 3.16.0
@wordpress/editor dependencies minor 9.13.0 -> 9.15.0
@wordpress/element dependencies minor 2.12.0 -> 2.13.1
@wordpress/format-library dependencies minor 1.15.0 -> 1.17.0
@wordpress/i18n dependencies minor 3.10.0 -> 3.12.0
@wordpress/icons dependencies minor 1.2.0 -> 1.4.0
@wordpress/is-shallow-equal dependencies major 1.8.0 -> 2.0.0
@wordpress/jest-preset-default dependencies major 5.5.0 -> 6.0.0
@wordpress/keyboard-shortcuts dependencies minor 1.2.0 -> 1.4.0
@wordpress/keycodes dependencies minor 2.10.0 -> 2.12.0
@wordpress/media-utils dependencies minor 1.8.0 -> 1.10.0
@wordpress/notices dependencies minor 2.1.0 -> 2.3.0
@wordpress/npm-package-json-lint-config dependencies major 2.2.0 -> 3.0.0
@wordpress/nux dependencies minor 3.13.0 -> 3.15.0
@wordpress/plugins dependencies minor 2.13.0 -> 2.15.0
@wordpress/primitives dependencies minor 1.2.0 -> 1.4.0
@wordpress/priority-queue dependencies minor 1.5.1 -> 1.6.0
@wordpress/redux-routine dependencies minor 3.7.0 -> 3.8.0
@wordpress/rich-text dependencies minor 3.13.0 -> 3.15.0
@wordpress/scripts dependencies major 7.2.0 -> 9.0.0
@wordpress/server-side-render dependencies minor 1.9.0 -> 1.11.0
@wordpress/shortcode dependencies minor 2.6.0 -> 2.7.0
@wordpress/url dependencies minor 2.12.0 -> 2.14.0
@wordpress/viewport dependencies minor 2.14.0 -> 2.16.0

Release Notes

WordPress/gutenberg

v2.9.0

Compare Source


Renovate configuration

📅 Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.


  • If you want to rebase/retry this PR, check this box

Closes #41899 (included)

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot requested a review from a team as a code owner April 17, 2020 06:05
@matticbot
Copy link
Contributor

@sirreal
Copy link
Member

sirreal commented Apr 20, 2020

#39624 is blocked on this.

@sirreal
Copy link
Member

sirreal commented Apr 20, 2020

Added @alshakero and @andrewserong. I think helping to review and land this fits in with create gardening work.

@sirreal
Copy link
Member

sirreal commented Apr 27, 2020

We can wait for these changes or assert some types until there's another package upgrade to include them:

Some incompatible type declarations have popped up.

@alshakero
Copy link
Member

alshakero commented Apr 27, 2020

This PR is failing to build for reasons similar to reasons here. I'll push a fix.

@renovate renovate bot requested a review from a team as a code owner April 27, 2020 15:53
@matticbot
Copy link
Contributor

matticbot commented Apr 27, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~16479 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-gutenboarding       +80231 B  (+4.5%)   +16452 B  (+3.4%)
entry-main                   +49 B  (+0.0%)      +10 B  (+0.0%)
entry-login                  +45 B  (+0.0%)      +12 B  (+0.0%)
entry-domains-landing        +21 B  (+0.0%)       +5 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~2607 bytes added 📈 [gzipped])

name              parsed_size           gzip_size
post-editor           +7748 B  (+0.4%)    +1479 B  (+0.3%)
checkout              +3562 B  (+0.3%)     +563 B  (+0.2%)
gutenberg-editor      +3406 B  (+0.4%)     +565 B  (+0.3%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@alshakero
Copy link
Member

alshakero commented Apr 27, 2020

Review 👀

Gutenboarding

I tested Gutenboarding under this update and all works great after the small fix I sent above.

I tested the flow from start to finish with as many state mutations as possible (changed domain, site title, vertical, design, etc..). Everything seems in order and the site was created without any issues.

FSE

I tested FSE too. I had some issues get it started initially but eventually got it running.

Template modal

The template modal works great. I picked a couple of templates and all rendered fine.

The FSE Experience

I created a page and played with around a dozen blocks and all works well. The only issue I noticed is that there are a few errors messages in the console log that read

Uncaught TypeError: Cannot read property 'cancelAnimationFrame' of null
    at RichText.componentWillUnmount (index.js:1466)
    at callComponentWillUnmountWithTimer (react-dom.js?ver=16.9.0:20004)
    at HTMLUnknownElement.callCallback (react-dom.js?ver=16.9.0:341)
    at Object.invokeGuardedCallbackDev (react-dom.js?ver=16.9.0:391)
    at invokeGuardedCallback (react-dom.js?ver=16.9.0:448)
    at safelyCallComponentWillUnmount (react-dom.js?ver=16.9.0:20011)
    at commitUnmount (react-dom.js?ver=16.9.0:20431)
    at commitNestedUnmounts (react-dom.js?ver=16.9.0:20487)
    at unmountHostComponents (react-dom.js?ver=16.9.0:20783)
    at commitDeletion (react-dom.js?ver=16.9.0:20852)

This might have been introduced in WordPress/gutenberg#21105 but I'm not sure. What I'm sure of though is that this issue is introduced in this update. Because RichText component is updated from 3.13.0 to 3.14.1 and the getWindow() method (where the issue originates) is created between these two versions according to the diff (can be seen here). Make sure to Find "packages/rich-text/src/component/index.js" file and load the diff, it isn't loaded by default.

Otherwise, everything seems to work OK.

Performance

The increase in the bundle size above

App Entrypoints (~14921 bytes added 📈 [gzipped])

It seems like a deal-breaker IMO.

@@ -14,7 +14,7 @@ $padding--gutenboarding__header: $grid-unit-10;
border-bottom: 1px solid $light-gray-500;
height: $gutenboarding-header-height;
background: $white;
z-index: z-index( '.block-editor-editor-skeleton__header' );
z-index: 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid hardcoding 😬

Suggested change
z-index: 1;
z-index: z-index( '.interface-interface-skeleton__header' );

These z-index changes normally come from Gutenberg renaming selectors, e.g. when moving a component to a different package. To find the relevant change, check the commit history of https://github.com/WordPress/gutenberg/blob/master/packages/base-styles/_z-index.scss.

In this case, the relevant change was here: https://github.com/WordPress/gutenberg/pull/21335/files#diff-1c24406914c18d0668eb1f64880d4144R27

@scinos
Copy link
Contributor

scinos commented Apr 28, 2020

FYI, there is also the renovate PR #41515 that will update some types of wordpress components.
Maybe we should merge #41515 into this PR?

@simison
Copy link
Member

simison commented Apr 28, 2020

This might have been introduced in WordPress/gutenberg#21105 but I'm not sure.

That's odd — so that change was slated for Gutenberg 7.9 and Simple sites are running 7.8. That shouldn't matter though since we don't really package these @wordpress/* deps into the FSE JS bundle. Those dependencies are "externalized" and we just use whatever is in wp-admin available.

(You might ask why to even test then, but it's still important we test the build since we rely on some@wordpress/*packages when building the bundle.)

@sirreal sirreal force-pushed the renovate/wordpress-monorepo branch from 84a125d to 25948bf Compare April 28, 2020 08:39
@sirreal
Copy link
Member

sirreal commented Apr 28, 2020

Rebased and fixed lockfile conflict by running yarn.

@sirreal
Copy link
Member

sirreal commented May 8, 2020

It would be good to confirm wpcom-FSE and SPT template selector also works.

I believe FSE (and SPT) depend WordPress installation-provided script dependencies. @wordpress/icons is the only exception. For the most part, this shouldn't affect FSE or other plugins that use external script dependencies.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the rebase @scinos. I've retested /new and I think this is good to go!

@scinos scinos merged commit e816f74 into master May 8, 2020
@scinos scinos deleted the renovate/wordpress-monorepo branch May 8, 2020 09:13
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 8, 2020
scinos added a commit that referenced this pull request May 8, 2020
scinos added a commit that referenced this pull request May 8, 2020
sirreal added a commit that referenced this pull request May 8, 2020
@sirreal
Copy link
Member

sirreal commented May 8, 2020

This was reverted when an error was discovered on staging. 2nd attempt in #41994

@sirreal sirreal added the Reverted This was removed after being merged. label May 8, 2020
scinos pushed a commit that referenced this pull request May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Reverted This was removed after being merged. [Type] Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants