-
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
Custom Editor: Initial Experiment #39373
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Webpack Runtime (~31149 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 (~2517811 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~6005357 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 (~2654497 bytes added 📈 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Moment.js Locales (~108669 bytes added 📈 [gzipped])
Locale data for moment.js. Unless you are upgrading the moment.js library, changes in these chunks are suspicious. 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. |
c5b9261
to
0790031
Compare
From what I can see this is still going in the direction of rebuilding the editor in Calypso from components, and my understanding was that our goal is much simpler now and that we should expose whatever existing in core's wp-admin Site Editor page now (for reference pbAok1-9M-p2#comment-623). I think the starting point should be much simpler now and consist of exposing the experimental Site Editor first in Dotcom's wp-admin (since it's hidden now) and then building up on top of that and showing it in Calypso (similar to Gutenframe). Let me know if that makes sense? |
🤷♂️ It makes sense; I just think we still consider different things simple and complex 😅 I was going to import edit-site into a new Calypso entrypoint for the sake of simpler developer experience: The entrypoint would allow running edit-site locally, and help us develop and iterate more quickly than if we're dealing with a full-blown wp-admin plugin (and thus, a WP install -- Dockerized, or WP.com sandbox). (We'd still be able to build a wp-admin plugin from this Calypso-integrated codebase.) It was my impression that whatever we need to solve for edit-site to work on WP.com, we'll face pretty much the same set of problems, regardless of whether we go the wp-admin plugin, or Calypso integration route. E.g.
Am I missing any big ones? Specifically any problems that present themselves much differently in a Calypso vs wp-admin context? I've also asked @mtias to weigh in on this question (and more generally, on pbAok1-6n-p2#comment-497 and paYE8P-jT). |
Some of these like "Set the correct REST API URL" are already solved by existing editor integration on Dotcom, and we won't have to tackle that separately as is the case here. I also don't think we should be building the "custom edit-site instance" any longer. That was the planned approach before, but now my understanding of the proposed direction is to just run the vanilla core site editor (which is in wp-admin natively), and extend it where necessary.
Any initialization logic that happens in PHP will have to be replicated with this. Admittedly, there is not as much of it for site editor now as for post edit, but it might grow in the future. Gutenberg functionality also assumes full page reloads in wp-admin, and that doesn't always translate well to Calypso's multisite realm. |
@@ -0,0 +1,8 @@ | |||
export const CUSTOM_EDITOR_SECTION_DEFINITION = { | |||
name: 'custom-editor', |
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.
In case we decide to proceed with this, I think this should be replaced with site-editor
or similar to avoid confusion.
though ideally, we would work to improve this ;) I do like the idea of going this route because it's certainly how we want to run it if everything worked in our favor. I just have a feeling that we would still run into all wp-admin issues we discussed when we talked about custom editor for pages (like theme CSS, php plugins, etc.) But maybe it is still worth exploring this if:
Since we'll be working closely on edit-site, maybe this is an opportunity to make sure that it's not tied to php or wp-admin as much as possible? |
That would be great but I'm not sure if it's possible. If we are going to expose 3rd party blocks in site editor it's ultimately going to depend on related plugins being loaded in wp-admin context? |
Definitely. I guess it depends if those blocks are loaded over the API endpoint rather than initialized on page load via PHP. (I'd be interested in finding out how this actually works; I don't really know.) |
229aa9c
to
3822742
Compare
In our meeting yesterday, we did discover that quite a bit of bootstrapping happens in php before loading the editor. Basically, the code which looks up the templates for the editor happens in php before returning the editor page. It was suggested this could be exposed behind a core API endpoint if we wanted to do a client-side editor. I think I still am not sure how we would expose 3rd party JS to a Calypso page. For example, the Jetpack JS/editor CSS bundles would have to be pulled in from wp-admin (same for any plugin). I'm not sure how we feel about that security-wise for 3rd party Atomic plugins, but I wonder if an endpoint could return URLs to the various JS/CSS bundles that need to be loaded into the editor. |
Is this up to date? I get a sensible |
Right, this should work now, per my latest couple of commits. Should've updated the PR desc -- will do now! |
Thanks, that's valuable information 👍
I think we can do that for the plugins that we provide ourselves, at least initially. (We might want to optimize against redundantly loading shared dependencies later.) For those plugins, there are also a few other options, such as packaging them as npms, and (statically or dynamically)
I like that thinking. This might even make sense in Core (could be an extensions of a potentially existing plugin list endpoint), and we could add a filter on WP.com that only whitelists the plugins provided by us (JP, CoBlocks). Alternatively, we could concoct a blocks endpoint for core (unless that's already a thing), which could expose information such as JS and CSS bundle locations as well as block availability -- for which we have some precedent in Jetpack. I know that a block availability endpoint, and dynamic block loading mechanism has been discussed in core before. As for security of third party plugins on Atomic sites, I still don't see any other solution but some sort of sandbox. As we're clearly not going to do that in the near term, I think that using an FSE that's built as a plugin for wp-admin is inevitable for Atomic sites 🙁 (Note to self: Still need to read p1HpG7-7po-p2 in case that's a viable alternative.) Really appreciate your input and feedback about this exploration, @noahtallen! |
838382b
to
c0db02a
Compare
I haven't seen that! Giving it a read now. It looks promising |
c0db02a
to
5082ea3
Compare
We already merged the initial version of wp-admin in #39877 so I think that we can abandon this approach for now. As already discussed we can easily reuse the existing wpcom customization for Gutenberg in wp-admin (just a matter of enqueueing appropriate scripts) while this path would require us recreate a good chunk of that functionality. In wp-admin we simply rely on existing PHP initialization code that we would have to expose via API endpoints if we went for the Calypso route. All of the above would require constant maintenance effort just to keep it running after Gutenberg plugin updates, especially since this area will be under active development in the upcoming period. |
Changes proposed in this Pull Request
Take two of #36771.
For context, see pbAok1-dh-p2#comment-589
I'm customizing
apiFetch
, based on D37093-code. This might be generic enough to make sense across Calypso -- allapiFetch
requests made there should go through the WP.com proxy, change the root topublic-api.wordpress.com
, add/sites/123456
, etc.TODO
request
fromwpcom-proxy-request
(rather than fromdata-stores
) once wpcom-proxy-request: Return Promise if no callback specified #39683 has been merged.Testing instructions
Go to http://calypso.localhost:3000/custom-editor
cc @Automattic/cylon