-
Notifications
You must be signed in to change notification settings - Fork 2k
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
base: trunk
Are you sure you want to change the base?
Conversation
@fullofcaffeine may be interested in this (#43970) |
eda435d
to
8fec0c3
Compare
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Webpack Runtime (~1000 bytes added 📈 [gzipped])
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])
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])
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])
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. Generated by performance advisor bot at iscalypsofastyet.com. |
I pushed a few changes. I think this achieves the best tradeoffs for this project:
Happy to chat about these changes. Love the efforts here, thanks! |
There was a problem hiding this 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.
b1a457d
to
b6817a5
Compare
@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? |
@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?
|
@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? |
@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). |
b6817a5
to
99360de
Compare
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 |
Thanks again @sirreal - looks like I got almost there but I had added some of the babel config in the |
81143d6
to
4591a65
Compare
@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. |
🎵 the listing error is out-of-scope and unrelated to changes made in this PR. |
@sgomes does this look familiar to you? it's failing to build calypso.live because of it thanks everyone in advance! 🙇♂️
|
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 Edit: I've been able to reproduce locally, with Edit 2: The issue seems to be during the phase where we analyse if the resulting fallback file only contains valid ES5 (the Edit 3: The issue seems to be the Edit 4: Solved. |
@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 Edit 2: Created a separate PR to fix the |
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 |
It's a dependency of the GitHub Issue block:
|
This PR has been marked as stale due to lack of activity within the last 30 days. |
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 withlerna
.Co-created with @annemirasol @mirka @brezocordero @lezama @unDemian @naxoc @mtias