Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Update wp.editor references #620

Closed
ryelle opened this issue Jun 10, 2019 · 6 comments · Fixed by #2434
Closed

Update wp.editor references #620

ryelle opened this issue Jun 10, 2019 · 6 comments · Fixed by #2434
Assignees
Labels
focus: blocks Specific work involving or impacting how blocks behave. type: refactor The issue/PR is related to refactoring.
Milestone

Comments

@ryelle
Copy link
Member

ryelle commented Jun 10, 2019

Needs a gutenberg reference, but some things(?) in wp.editor are moving to wp.blockEditor, and it's throwing warnings with the latest version of the gb plugin. Will need to see about backwards compat, but for example:

wp.editor.BlockControls is deprecated and will be removed. Please use wp.blockEditor.BlockControls instead.

wp.editor.InspectorControls is deprecated and will be removed. Please use wp.blockEditor.InspectorControls instead.

wp.editor.InnerBlocks is deprecated and will be removed. Please use wp.blockEditor.InnerBlocks instead.

@ryelle ryelle added the WP Core label Jun 10, 2019
@mikejolley mikejolley added the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Jul 9, 2019
@nerrad
Copy link
Contributor

nerrad commented Jul 28, 2019

Related pull where changes were introduced: WordPress/gutenberg#14112
Related dev doc: https://make.wordpress.org/core/2019/04/09/the-block-editor-javascript-module-in-5-2/

@nerrad
Copy link
Contributor

nerrad commented Sep 26, 2019

I don't think this should be in the icebox. While the warnings are deprecated warnings, with the pending release of WordPress 5.3 we'll soon have two major versions of WordPress out on the wild where Woo Blocks will throw deprecation warnings because of our incorrect usage of imports from @wordpress/editor.

We have a couple options:

  • wait until WooCommerce core updates minimum WordPress version required to 5.2+
  • Create our own wrapper bundle for the wp.blockEditor external and if that's not present, export wp.editor instead. Then we can alias all imports in our own scripts to that export (@woocommerce/block-editor-compat).

The first option is the easiest of course and results in less code churn. But it does mean that we are seeing these deprecated notices for a while. Since merging @woocommerce/admin to Woo Core will be predicated by a minimum version bump to WordPress 5.2, this means this option is more viable.

The second option is doable and would have the immediate benefit of ensuring we're always using the correct imports for whatever environment the blocks is loaded in (and thus eliminating the deprecation notices). The downsides are the code-churn for replacing all existing imports and the potential confusion with the seeing @woocommerce/block-editor-compat as imports.

I'm leaning towards the first option but I also could probably implement the second option in half a day to a day's work.

At a minimum though, I think this should be in the backlog pipeline, not the icebox - so I've moved it if that's okay?

cc @pmcpinto

@Aljullu
Copy link
Contributor

Aljullu commented Sep 27, 2019

The first option also seems the best to me, we avoid polluting the code and having to make the change twice. It comes with the drawback that we will get warnings for some time, but I guess most users will never notice that.

In case it helps, there was some discussion about this in: paAmJe-tx-p2.

@pmcpinto
Copy link

At a minimum though, I think this should be in the backlog pipeline, not the icebox - so I've moved it if that's okay?

@nerrad thanks for the explanation. I'm ok with moving it to the backlog. I think it was Mike that moved it to the Icebox, I don't know what was the reason behind that.

@nerrad nerrad added type: refactor The issue/PR is related to refactoring. focus: blocks Specific work involving or impacting how blocks behave. and removed status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. wordpress-core labels Jan 24, 2020
@mikejolley
Copy link
Member

@nerrad just raising this again for your 👀 - I think we talked about this min version being bumped for our next release, so that would be a good time to fix this.

@nerrad
Copy link
Contributor

nerrad commented May 6, 2020

OH yaaa. It would be a good time! Let's do it. It doesn't increase scope that much because it's mostly just correcting what package (external) components point to. Will be nice to get rid of the warning :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: blocks Specific work involving or impacting how blocks behave. type: refactor The issue/PR is related to refactoring.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants