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

DevDocs: Add block playground #44153

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
Open

DevDocs: Add block playground #44153

wants to merge 10 commits into from

Conversation

dmsnell
Copy link
Contributor

@dmsnell dmsnell commented Jul 14, 2020

Adds a new playground in the DevDocs section to streamline developing o2-blocks entirely inside Calypso without any WordPress instance and without building the package with lerna.

Co-created with @annemirasol @mirka @brezocordero @lezama @unDemian @naxoc @mtias

@matticbot
Copy link
Contributor

@sirreal
Copy link
Member

sirreal commented Jul 15, 2020

@fullofcaffeine may be interested in this (#43970)

@dmsnell dmsnell changed the title (WIP) DevDocs: Add block playground DevDocs: Add block playground Jul 15, 2020
@dmsnell dmsnell added [Goal] Gutenberg Working towards full integration with Gutenberg [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Blocks Editor blocks, aka Gutenberg blocks, plugins, and extensions DevDocs labels Jul 15, 2020
@matticbot
Copy link
Contributor

matticbot commented Jul 16, 2020

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

Webpack Runtime (~1000 bytes added 📈 [gzipped])

name      parsed_size           gzip_size
manifest      +3531 B  (+1.7%)    +1000 B  (+2.4%)

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 (~43429 bytes added 📈 [gzipped])

name                   parsed_size            gzip_size
entry-gutenboarding      +199399 B  (+11.4%)   +43483 B  (+9.5%)
entry-main                  +305 B   (+0.0%)      +32 B  (+0.0%)
entry-domains-landing        -60 B   (-0.0%)      -86 B  (-0.1%)

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

Sections (~54636 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
o2-blocks                +229372 B    (new)   +51475 B    (new)
gutenberg-editor           +2974 B  (+0.3%)     +214 B  (+0.1%)
devdocs                    +1458 B  (+0.7%)     +150 B  (+0.3%)
checkout                   +1238 B  (+0.1%)       +0 B
jetpack-connect             +724 B  (+0.1%)     +113 B  (+0.1%)
post-editor                 +653 B  (+0.0%)      -27 B  (-0.0%)
media                       +138 B  (+0.0%)     +729 B  (+0.6%)
themes                      +132 B  (+0.0%)     +893 B  (+0.8%)
earn                        +106 B  (+0.0%)    +1010 B  (+1.3%)
settings                     -84 B  (-0.0%)      -30 B  (-0.0%)
stats                        +36 B  (+0.0%)       +9 B  (+0.0%)
plugins                      +30 B  (+0.0%)       +4 B  (+0.0%)
reader                       +24 B  (+0.0%)       +8 B  (+0.0%)
people                       +18 B  (+0.0%)       +4 B  (+0.0%)
domains                      +18 B  (+0.0%)       -3 B  (-0.0%)
comments                     +18 B  (+0.0%)       -6 B  (-0.0%)
site-blocks                  +12 B  (+0.0%)       +2 B  (+0.0%)
notification-settings        +12 B  (+0.0%)       +2 B  (+0.0%)
import                       +12 B  (+0.0%)       +2 B  (+0.0%)
google-my-business           +12 B  (+0.0%)       +7 B  (+0.0%)
settings-writing             +11 B  (+0.0%)      +80 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 (~455756 bytes added 📈 [gzipped])

name                                                         parsed_size           gzip_size
async-load-block-playground                                   +1798677 B    (new)  +455713 B    (new)
async-load-layout-masterbar-checkout                             +1210 B  (+0.5%)      +63 B  (+0.1%)
async-load-design-playground                                       -35 B  (-0.0%)       +5 B  (+0.0%)
async-load-design                                                  -35 B  (-0.0%)      +77 B  (+0.0%)
async-load-post-editor-media-modal                                 -27 B  (-0.0%)     -123 B  (-0.2%)
async-load-signup-steps-domains                                    +18 B  (+0.0%)       +6 B  (+0.0%)
async-load-layout-guided-tours-component                           +18 B  (+0.0%)       +2 B  (+0.0%)
async-load-layout-guided-tours                                     +18 B  (+0.1%)       +3 B  (+0.1%)
async-load-extensions-woocommerce-app-store-stats-referrers        +12 B  (+0.0%)       +4 B  (+0.0%)
async-load-components-web-preview-component                        +12 B  (+0.0%)       +6 B  (+0.0%)

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.

@sirreal
Copy link
Member

sirreal commented Jul 16, 2020

I pushed a few changes. I think this achieves the best tradeoffs for this project:

  • Remove the React imports and webpack config. Calypso build does the right thing, we don't need to provide an explicit config.
  • Use a babel override to handle automatic imports for React in o2-blocks. This allows us to omit the explicit createElement imports (common practice in Gutenberg). This way we the implicit imports will work for JSX -> wp.element.createElement or React.createElement appropriately when building for Calypso or for WP Admin.

Happy to chat about these changes. Love the efforts here, thanks!

Copy link
Contributor

@annemirasol annemirasol left a comment

Choose a reason for hiding this comment

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

I've been playing a lot with it on calypso.live, and I think this is good to merge!

Some UI things, like the editor has left padding which makes it unaligned with the code preview window, and other stuff can be addressed in iterations.

@dmsnell dmsnell force-pushed the add/block-playground branch 2 times, most recently from b1a457d to b6817a5 Compare August 10, 2020 20:20
@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 10, 2020

@Automattic/team-calypso any idea why the linter is rejecting this for a url import when I don't touch that part of the controller?

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 10, 2020

@sirreal because you've been such a big help I'm reluctant to bother you again but I rebased against the latest and now the o2-blocks aren't building. looks like it could be a webpack issue - are you aware of any changes that have happened in Calypso in the past few weeks that would require us to change the webpack patch you submitted?

ERROR in chunk view [entry]

https://app.circleci.com/pipelines/github/Automattic/wp-calypso/70048/workflows/bb37bf7f-90fc-4c92-a0d8-15bd55efbc7f/jobs/845891

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 12, 2020

@sirreal after more investigation I think I lost your commit somehow. this and its related branch were messed up and I tried to clean them but now I can't find any reference to your commit. do you by any chance still have a copy of the branch on your computer?

@sarayourfriend
Copy link
Contributor

@dmsnell We run the linter on the entire file that was modified, not just the lines modified. If it's out of scope for this PR to fix that error then I think you can just ignore it (I usually add a note in my PR description that the particular linter failures are out of scope).

@sirreal
Copy link
Member

sirreal commented Aug 17, 2020

I've pushed the same changes I think I'd made before. The builds appear to be working. I haven't tested o2-blocks in the editor, although I expect them to be fine as that part of the build is now unchanged. In Calypso, the build works but there there is an error when I click the quick inserter likely related to changes in Gutenberg with the quick inserter and patterns.

For the record, Calypso babel changes are based on 924fb2c

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 17, 2020

Thanks again @sirreal - looks like I got almost there but I had added some of the babel config in the o2-blocks directory when I guess I needed to add it to the root repo.

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 17, 2020

there is an error when I click the quick inserter likely related to changes in Gutenberg with the quick inserter and patterns

@sirreal I think the latest Gutenberg update broke this. @saramarcondes noticed this as well in #44801. I don't believe it's due to these changes or introduced in them.

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 17, 2020

🎵 the listing error is out-of-scope and unrelated to changes made in this PR.

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 17, 2020

@sgomes does this look familiar to you? it's failing to build calypso.live because of it
builds fine locally cc: @Automattic/team-calypso

thanks everyone in advance! 🙇‍♂️

[fallback ] /calypso/public/fallback/131.617c19d088da8347a878.min.js
[fallback ]   1:72403  error  Parsing error: Parenthesized pattern
[fallback ] 
[fallback ] ✖ 1 problem (1 error, 0 warnings)
[fallback ] 

@sgomes
Copy link
Contributor

sgomes commented Aug 18, 2020

@sgomes does this look familiar to you? it's failing to build calypso.live because of it
builds fine locally cc: @Automattic/team-calypso

thanks everyone in advance! 🙇‍♂️

[fallback ] /calypso/public/fallback/131.617c19d088da8347a878.min.js
[fallback ]   1:72403  error  Parsing error: Parenthesized pattern
[fallback ] 
[fallback ] ✖ 1 problem (1 error, 0 warnings)
[fallback ] 

That's a new one for me; can't say I've ever come across anything like that, nor does it make sense to me why it would fail on calypso.live but not locally.

Edit: I've been able to reproduce locally, with CALYPSO_ENV=production NODE_ENV=production yarn build. Investigating.

Edit 2: The issue seems to be during the phase where we analyse if the resulting fallback file only contains valid ES5 (the validate-fallback-es5 script).

Edit 3: The issue seems to be the new-github-issue-url npm module now being bundled, which contains ES6 syntax (namely, arrow functions). It will need to be added to the list of modules to be transpiled during build. I'll push a commit with that change.

Edit 4: Solved.

@sgomes
Copy link
Contributor

sgomes commented Aug 18, 2020

⚠️ Performance warning ⚠️

@dmsnell This PR seems to be adding 200KB of JS to the Gutenboarding critical path. That is a massive amount of JS, and I don't think this PR should go live while that's the case. I'm guessing it's an oversight somewhere, since this is supposed to be a DevDocs change?

Edit: The bundle seems to be growing because several large things are being added to entry-gutenboarding, the largest among them being react-dom. core-js is another big offender here. I'm guessing there's a React version mismatch somewhere between Calypso and @wordpress/ that is caused by one of the @wordpress modules being included in this PR? CC @sirreal

Edit 2: Created a separate PR to fix the react/react-dom duplication: #44987

@dmsnell
Copy link
Contributor Author

dmsnell commented Aug 18, 2020

This PR seems to be adding 200KB of JS to the Gutenboarding critical path. That is a massive amount of JS, and I don't think this PR should go live while that's the case. I'm guessing it's an oversight somewhere, since this is supposed to be a DevDocs change?

Thanks @sgomes - I'll look into that and try to make sure it doesn't bloat Calypso. thanks also for your guidance on the build failures. I don't know what new-github-issue-url is or why it's related to this branch. I guess it's some transitive dependency of our babel transform? or some block int he o2-blocks?

@sirreal
Copy link
Member

sirreal commented Aug 19, 2020

I don't know what new-github-issue-url is or why it's related to this branch. I guess it's some transitive dependency of our babel transform? or some block int he o2-blocks?

It's a dependency of the GitHub Issue block:

import * as createIssueUrl from 'new-github-issue-url';

@sarayourfriend sarayourfriend changed the base branch from master to trunk November 20, 2020 16:12
@github-actions
Copy link

github-actions bot commented May 5, 2021

This PR has been marked as stale due to lack of activity within the last 30 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocks Editor blocks, aka Gutenberg blocks, plugins, and extensions DevDocs [Goal] Gutenberg Working towards full integration with Gutenberg [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants