Skip to content

Conversation

@ockham
Copy link
Contributor

@ockham ockham commented Dec 29, 2019

Changes proposed in this Pull Request

and remove local copy of Card component.

PR based on #38554. Rebase branch (and base PR on master) once that PR has been merged.

Testing instructions

Verify that Gutenboarding still works.

@ockham ockham added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial [Goal] Gutenberg Working towards full integration with Gutenberg [Feature Group] Signup & Site Onboarding Tools for user registration and onboarding new users to the site. labels Dec 29, 2019
@ockham ockham requested a review from a team December 29, 2019 14:01
@ockham ockham self-assigned this Dec 29, 2019
@matticbot
Copy link
Contributor

@ockham ockham force-pushed the remove/gutenboarding-local-card-component-copy branch from 4636dc0 to 8445ed3 Compare December 29, 2019 15:35
@matticbot
Copy link
Contributor

matticbot commented Dec 29, 2019

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

Webpack Runtime (~59 bytes removed 📉 [gzipped])

Details
name      parsed_size           gzip_size
manifest       -901 B  (-0.5%)      -59 B  (-0.2%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

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

Details
name                 parsed_size           gzip_size
entry-gutenboarding      -1223 B  (-0.1%)    +3371 B  (+0.7%)

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

Sections (~29858 bytes removed 📉 [gzipped])

Details
name              parsed_size            gzip_size
theme               -139480 B  (-35.3%)   -30798 B  (-31.5%)
themes                -2785 B   (-0.9%)     +329 B   (+0.4%)
plugins               -2785 B   (-0.6%)     +329 B   (+0.3%)
hosting               -2785 B   (-1.2%)     +329 B   (+0.5%)
import                -2397 B   (-1.2%)     -655 B   (-1.2%)
gutenberg-editor      +1593 B   (+0.2%)     +460 B   (+0.2%)
checkout               +810 B   (+0.1%)     +176 B   (+0.1%)
signup                 -664 B   (-0.4%)      -28 B   (-0.1%)

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

Async-loaded Components (~2792 bytes removed 📉 [gzipped])

Details
name                                           parsed_size            gzip_size
async-load-signup-steps-import-url-onboarding      -2538 B   (-7.5%)    -1442 B  (-13.8%)
async-load-signup-steps-import-url                 -2538 B  (-10.9%)    -1350 B  (-18.9%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.

@ockham
Copy link
Contributor Author

ockham commented Dec 29, 2019

The TypeScript check is currently failing since this also needs an update of @types/wordpress__components (to include Card, CardMedia, and friends) -- possibly upstream.

@ockham
Copy link
Contributor Author

ockham commented Jan 6, 2020

The TypeScript check is currently failing since this also needs an update of @types/wordpress__components (to include Card, CardMedia, and friends) -- possibly upstream.

Just checked https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/wordpress__components/, and nope, card ain't there. Nor could I find a PR or issue.

Do we have infrastructure in Gutenberg in place already that produces typedefs for us? Haven't kept track lately 😬 /cc @sirreal @gziolo @aduth

@ockham
Copy link
Contributor Author

ockham commented Jan 6, 2020

Do we have infrastructure in Gutenberg in place already that produces typedefs for us? Haven't kept track lately /cc @sirreal @gziolo @aduth

Ah here: WordPress/gutenberg#18942.

@ockham
Copy link
Contributor Author

ockham commented Jan 7, 2020

Adding type defs for Card here: DefinitelyTyped/DefinitelyTyped#41448 (WIP)

@sirreal sirreal force-pushed the renovate/wordpress-monorepo branch from 6792f34 to 2b76cdd Compare January 8, 2020 13:59
@sirreal sirreal force-pushed the remove/gutenboarding-local-card-component-copy branch from 8445ed3 to 5a86e07 Compare January 8, 2020 14:03
@sirreal
Copy link
Member

sirreal commented Jan 8, 2020

Rebased and pushed an improvement to align with types in DefinitelyTyped/DefinitelyTyped#41448

@sirreal sirreal force-pushed the remove/gutenboarding-local-card-component-copy branch from 5a86e07 to 7e6fa86 Compare January 8, 2020 21:49
@sirreal
Copy link
Member

sirreal commented Jan 8, 2020

Rebased

@sirreal sirreal force-pushed the renovate/wordpress-monorepo branch from c8ae564 to 00a4d60 Compare January 10, 2020 11:56
@sirreal sirreal force-pushed the remove/gutenboarding-local-card-component-copy branch from dd3505d to 285849a Compare January 10, 2020 12:02
@sirreal
Copy link
Member

sirreal commented Jan 10, 2020

Rebased

@sirreal sirreal force-pushed the renovate/wordpress-monorepo branch from b4f0039 to e1f9673 Compare January 13, 2020 11:00
@renovate renovate bot closed this Jan 13, 2020
@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 Jan 13, 2020
@sirreal
Copy link
Member

sirreal commented Jan 13, 2020

With merge/rebase/auto-closing/changing target branch, this got into a state where I couldn't open or change the base branch…

I've move it to a new PR: #38802

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature Group] Signup & Site Onboarding Tools for user registration and onboarding new users to the site. [Goal] Gutenberg Working towards full integration with Gutenberg [Status] Blocked / Hold [Type] Janitorial

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants