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

[Experimental] Gutenberg diff: add plugin for cherry-picking new features #32624

Closed
wants to merge 1 commit into from

Conversation

vindl
Copy link
Member

@vindl vindl commented Apr 26, 2019

Closes #32291

DO NOT MERGE

Experimental PR aiming to assess the feasibility of cherry-picking new Gutenberg features and running them on top of older Core version. This is relying on the same approach that we used for Full Site Editing plugin.

This PR is using a recently introduced Group block for initial PoC: WordPress/gutenberg#13964
WordPress/gutenberg#14920

Note: not all of the relevant code has been carried over (some theme styling is missing).

Testing instructions

  1. Sandbox your test site.
  2. Apply D27522-code to your sandbox.
  3. From your sandbox, run ./bin/calypso/fetch-gutenberg-diff.sh 265157 (Replace with CircleCI job ID for this PR if needed).
  4. Navigate to https://wordpress.com/block-editor/post/ and select your sandboxed site from step 1.
  5. You should be able to insert the Group block which is currently not available on WPCOM.

@matticbot
Copy link
Contributor

@vindl vindl force-pushed the try/gutenberg-diff branch from 8a71f56 to 4f92fb0 Compare April 26, 2019 21:54
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@vindl vindl force-pushed the try/gutenberg-diff branch from 9d392fb to cbfde41 Compare April 26, 2019 23:07

import './style.scss';

export const name = 'gutenberg-diff/group';
Copy link
Member Author

Choose a reason for hiding this comment

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

This should have the same name as the core block, but I just used something different for easier testing alongside the proper Group block on my local setup that's running latest Gutenberg master.

@vindl
Copy link
Member Author

vindl commented Apr 26, 2019

After playing around with this for a bit I'm not sure I can recommend this approach going forward. As soon as we get into porting some realistic feature/block we get into the messy business of unraveling the dependency web and trying to build hacks to mock some parts of it (e.g. CSS variables and mixins). Also the code can't be copied verbatim and some adjustments need to be made on case-by-case basis (e.g. here the @wordpress/block-editor dependency had to be replaced with @wordpress/editor since that package was recently renamed and is not available on Dotcom).

We also have to pin down the package versions to older ones that correspond with the core version that we are running.

@vindl vindl added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Apr 26, 2019
@vindl vindl requested a review from a team April 26, 2019 23:16
@vindl vindl force-pushed the try/gutenberg-diff branch from cbfde41 to 8290cfc Compare April 26, 2019 23:22
@dmsnell
Copy link
Member

dmsnell commented Apr 27, 2019

Nice experiment. I'd have been skeptical of a more positive response but I am glad we have some actual experience trying to do this to understand the option better.

I'm not sure I can recommend this approach going forward

Where does that leave you? Recommending against cherry-picking altogether or do you have an alternative approach?

@vindl
Copy link
Member Author

vindl commented Apr 28, 2019

Where does that leave you? Recommending against cherry-picking altogether or do you have an alternative approach?

@dmsnell I'm suggesting that we stick to the ~2-month core release cycle that we have on Dotcom now. With our current development process we are not blocked by Dotcom's version anymore, since we are developing a plugin that we are testing on local WP installs. If we want to test it on Dotcom during development, we can just pull latest Gutenberg to our sandbox without committing (can be automated afterwards).

Our work so far has shown that we can build most of the stuff that we need with what's already in Dotcom's core version. On top of that, this decision is easy to revert (zero effort, no cleanup needed) - if we deem it necessary, we can always switch to some other approach down the road, so it seems safest in that regard to me.

To summarize, my overall suggested strategy would be:

  • don't use the plugin or cherry-picking on Dotcom, stick to what we get with regular core updates.
  • account for this slower cadence with better project planning (e.g front-loading the core work etc).
  • in case of critical issues that can't wait for the core merge, temporarily fall back to the cherry-picking method suggested above.

In my opinion, all of the suggested approaches so far carry some drawbacks, but to me it seems that this one has the lowest amount and it's the easiest one move away from in the future.

@Copons
Copy link
Contributor

Copons commented Apr 29, 2019

Thanks for this exploration @vindl!
It's an interesting approach and it shows how crazy hard would cherry-picking be.

in case of critical issues that can't wait for the core merge, temporarily fall back to the cherry-picking method suggested above.

I'm struggling to think of a case when this would happen.
If the critical issue is on Core side, I'm confident Core folks will ship a point release ASAP.
If it's on our side, why would we need a Core update instead of fixing (or disabling) our code?

Also let's not forget that features not yet merged into Core should not be considered final (see the Section block), and so we should avoid them as much as we can.

@vindl
Copy link
Member Author

vindl commented Apr 29, 2019

I'm struggling to think of a case when this would happen.
If the critical issue is on Core side, I'm confident Core folks will ship a point release ASAP.
If it's on our side, why would we need a Core update instead of fixing (or disabling) our code?

Hopefully never. My original idea was - something that's not critical enough for core point release, but is annoying our users and HEs, so we want to provide a temporary workaround until the next core merge. It doesn't have be isolated to core issue either - let's say one of our features requires some core change/functionality (so it's not a problem for vanilla Gutenberg). Again, all of this probably wouldn't require a separate plugin, but could be provided as temporary band-aids within the FSE plugin. This is all assuming that we can use Gutenberg's API and extension points to implement the fix - if the problem is behind that line of abstraction, we'd have to introduce temporary core hacks in WPCOM (something that I'd really like to avoid).

@vindl
Copy link
Member Author

vindl commented Apr 29, 2019

Closing with the following proposal #32624 (comment).

@vindl vindl closed this Apr 29, 2019
@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 Apr 29, 2019
@Copons Copons deleted the try/gutenberg-diff branch April 30, 2019 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gutenberg: research spike for pulling in latest features to wpcom [3]
4 participants