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 API's audit 5.4 Release #20116

Closed
1 of 18 tasks
jorgefilipecosta opened this issue Feb 8, 2020 · 19 comments
Closed
1 of 18 tasks

Experimental API's audit 5.4 Release #20116

jorgefilipecosta opened this issue Feb 8, 2020 · 19 comments
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Type] Project Management Meta-issues related to project management of Gutenberg

Comments

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Feb 8, 2020

In this issue, I list the experimental API's we have in the editor. I document some decisions to stabilize or not, and I ask inputs to contributors of the experimental API's in case the decision of stabilization is not clear or a change may be possible.

In case I'm missing experimental API's or there is feedback to some decision, please leave a comment.

Thank you in advance for any inputs we could provide!

Block Library

  • __experimentalRegisterExperimentalCoreBlocks: The function is used to register experimental blocks so it should continue to be experimental (it is not included in core).

Blocks

Core data

  • __experimentalUseEntitySaving: Hi @epiqueras, this hook is not being used anywhere. Could we remove it?

Data

  • __experimentalResolveSelect: We have some cases but the possibility of using this wrong without a true need is very real. I think it would be good to think if there are better ways to accomplish equivalent objectives. I don't think for now we should offer this mechanism as stable.

Date

  • __experimentalGetSettings: The ultimate objective is to remove the moment library so for now, this function should continue to be experimental.

RichText

  • __experimentalRichText: The component is meant to separate RichText and settings specifically for the block editor, but the underlying component is not ready to be exposed.

Element

  • __experimentalCreateInterpolateElement: Hi @nerrad, should this mechanism be stabilized?

Compose

  • __experimentalUseDragging: Should only be stabilized after getting wider usage inside the editor itself. Current drag & drop iterations should be refactored to use then hook so we can verify if it is useful

  • useViewportMatch.__experimentalWidthProvider: It should only be stabilized after the intended use case (mobile view in customizer) uses it with success.

Components

The UI for color and gradients is going to suffer some changes in the future with the new design work in progress, so the new gradient related components will not be stabilized:

  • __experimentalGradientPicker
  • _experimentalCustomGradientPicker

Blocks can still use the components in the experimental form needed.

Block Editor

Components

  • __experimentalBlockNavigationList: Hi @talldan, should this be stabilized?
  • __experimentalBlockVariationPicker: Hi @gziolo, the block variations is a feature of 5.4, should this component be stable?
  • __experimentalEditorSkeleton: Very recently merged as experimental, it seems it should continue as experimental. cc: @ockham
  • __experimentalImageSizeControl: The way we deal with image sizes across different blocks is still being iterated it seems wise to keep the component experimental for now. cc: @ryelle
  • __experimentalLinkControl: Hi @getdave, @aduth, is link control ready to be exposed?
  • __experimentalResponsiveBlockControl: Component not yet used on core should continue experimental until its possible use cases are finished. @getdave is this component still needed?
  • __experimentalImageURLInputUI: The work to use this component across media blocks is not finished it seems the component should continue as experimental for now.
  • __experimentalBlockSettingsMenuFirstItem and __experimentalBlockSettingsMenuPluginsExtension: It seems the API to extend block settings did not evolved yet. Could you confirm @youknowriad?
  • __experimentalInserterMenuExtension: This API is supposed to be used by the block library and the block library is not yet part of WP 5.4 so I think we should keep it as experimental.

Mobile-only components

These components seem to be only used on mobile so I think they should continue experimental until equivalent usages on the web exist:

  • __experimentalWithPageTemplatePickerVisible

  • __experimentalUsePageTemplatePickerVisible

  • __experimentalBlockListFooter

  • __experimentalPageTemplatePicker

  • @koke could you that these components are only used in the mobile native apps? Would it be possible to just export them on the index.native.js file?

Color and gradient related functionality

  • __experimentalGradientPicker
  • __experimentalGradientPickerControl
  • __experimentalGradientPickerPanel
  • __experimentalColorGradientControl
  • __experimentalPanelColorGradientSettings
  • __experimentalUseColors
  • __experimentalGetGradientClass
  • __experimentalGetGradientObjectByGradientValue
  • __experimentalUseGradient

As referred for the components counterparts, the UI for color and gradients is going to suffer some changes in the future with the new design work in progress, so the new color & gradient related components will not be stabilized in WordPress 5.4.

Selectors

  • __experimentalGetAllowedBlocks: Hi @talldan should we stabilize this selector? Does it fit a need that an external developer may have that canInsertBlockTyle or checking the inner blocks settings would not fit?
  • __experimentalGetBlockListSettingsForBlocks: It seems this selector was used as part of the mechanism to render the child toolbar on the parent. That mechanism is not having considerable use cases yet, so I think the selector should continue as experimental.
  • __experimentalGetParsedReusableBlock: Hi @youknowriad, should this selector be stabilized?
  • __experimentalGetLastBlockAttributeChanges: Hi @aduth, this selector was merged for some time is it ready to be stable?

Settings

All the experimental settings refer to experimental block editor features or very specific WordPress needs that don't fit in a generic block editor so it seems this setting should continue as experimental:

  • __experimentalCanUserUseUnfilteredHTML
  • __experimentalEnableLegacyWidgetBlock
  • __experimentalBlockDirectory
  • __experimentalEnableFullSiteEditing
  • __experimentalEnableFullSiteEditingDemo
  • __experimentalEnablePageTemplates

Editor

Actions

  • __experimentalLocalAutosave: To be stabilized by API: Stabilize localAutosave() as autosave( { local: true } ) #20149

  • __experimentalTearDownEditor: @aduth can the action be stabilized?

  • @noisysocks Reusable block related actions have been experimental for a year, I think with FSE coming stabilizing in 5.4 is not a good bet. Should we keep experimental in 5.4 and stabilize for 5.5 given the FSE efforts will be more advanced?

  • __experimentalFetchReusableBlocks

  • __experimentalReceiveReusableBlocks

  • __experimentalSaveReusableBlock

  • __experimentalDeleteReusableBlock

  • __experimentalUpdateReusableBlock

  • __experimentalConvertBlockToReusable

  • __experimentalConvertBlockToStatic

  • @nerrad similar to reusable block would it make sense to keep this function as experimental and stabilise in 5.5? Do this API's still fit well with the developments we had in the entities mechanism?

  • __experimentalRequestPostUpdateStart

  • __experimentalRequestPostUpdateFinish

  • __experimentalOptimisticUpdatePost

@jorgefilipecosta jorgefilipecosta added [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Project Management Meta-issues related to project management of Gutenberg labels Feb 8, 2020
@epiqueras
Copy link
Contributor

epiqueras commented Feb 8, 2020 via email

@youknowriad
Copy link
Contributor

@epiqueras yes, make sense to me

@youknowriad
Copy link
Contributor

__experimentalSlotFillConsumer

I don't like this API. I don't have an alternative though, so I'd prefer if we keep it as is for sometime or rename it as "unsable" (something that will stay unstable forever)

__experimentalBlockSettingsMenuPluginsExtension

This is the slot to add items to the block settings popover right? It feels like it deserves to be made stable? thoughts? maybe the name is not great?

__experimentalGetParsedReusableBlock

This shouldn't be stabilized yet, we need to refactor the reusable block to use entities first and see if it's still useful.

@talldan
Copy link
Contributor

talldan commented Feb 10, 2020

Hey @jorgefilipecosta:

__experimentalGetBlockLabel, __experimentalGetAccessibleBlockLabel

These will probably be changed to selectors in #19664. That PR still needs some love though. Also some discussion on combining them in that PR, but some counter-arguments to that too.

__experimentalBlockNavigationList

This one could also change quite a bit if I can get #18014 moving forwards, so it shouldn't be stabilized.

__experimentalGetAllowedBlocks

@draganescu implemented this one, though I made some changes to it later.

This is used for auto-inserting a block from an appender when there's only one allowed block. The selector is only used in one place but seems fairly stable.

I think it's a bit more complicated than checking the block list settings. Depending on how the selector is used, it also has to check for templateLock—basically everything that canInsertBlockTypeUnmemoized does. Potentially I supposed it should be renamed to __experimentalGetInsertableBlocks if that's unclear. There is also getInserterItems which is similar, but does a lot more in terms of ordering/categorizing blocks, which is not needed for this use case.

@koke
Copy link
Contributor

koke commented Feb 10, 2020

@koke could you that these components are only used in the mobile native apps? Would it be possible to just export them on the index.native.js file?

Those weren't really mobile specific, the first version of the page template picker started as a multi-platform experiment, but there is still ongoing discussion about how to handle templates layouts on the web, so we've focused on mobile for now. I think it makes sense to clean up and remove the web bits until there's a clearer direction for the web layout picker.

@gziolo
Copy link
Member

gziolo commented Feb 10, 2020

__experimentalBlockSettingsMenuFirstItem and __experimentalBlockSettingsMenuPluginsExtension: It seems the API to extend block settings did not evolved yet. Could you confirm @youknowriad?

This is the slot to add items to the block settings popover right? It feels like it deserves to be made stable? thoughts? maybe the name is not great?

Yes, those are SlotFill for the block settings dropdown menu. We should rethink this approach and promote it to a stable API. Do you want me to take care of it?

@aduth
Copy link
Member

aduth commented Feb 10, 2020

__experimentalLinkControl: Hi @getdave, @aduth, is link control ready to be exposed?

I do not feel it is ready at this time. There is ongoing discussion at #18061 which may warrant future breaking API changes and/or adjustments of package location.

__experimentalGetLastBlockAttributeChanges: Hi @aduth, this selector was merged for some time is it ready to be stable?

I may want to look back to its original history, but while I know there was some undesirable (unstable) actions and state related to pending, previous, and "persistent" block changes, the fact that this one is marked as experimental leads me to believe that it could be relatively safe to stabilize.

__experimentalTearDownEditor: @aduth can the action be stabilized?

I don't strictly see it being problematic on its own, but given that this is only used as part of the editor "readiness" state, and the corresponding selector __unstableIsEditorReady is marked as unstable (meaning it would not be desirable to ever stabilize), I think it should be blocked by a decision to remove or stabilize that unstable selector.

@mcsf
Copy link
Contributor

mcsf commented Feb 10, 2020

* `__experimentalLocalAutosave`: @mcsf can the action be stabilized?

Andrew and I discussed this just two weeks ago: #19915 (comment)

  • __experimentalUpdateLocalAutosaveInterval: One takeaway was that we could allow some extra time to see if the proposed Features API could influence this sort of config.

  • __experimentalLocalAutosave: It's likely fine to stabilise.

@ockham
Copy link
Contributor

ockham commented Feb 11, 2020

__experimentalCreateInterpolateElement: Hi @nerrad, should this mechanism be stabilized?

Calypso developers have started using this function, and we would like to continue to do so, as it's one of the missing pieces that'll allow us standardizing on @wordpress/i18n in favor of i18n-calypso.

My vote for stabilizing!

/cc @sirreal @jsnajdr @akirk

@ockham
Copy link
Contributor

ockham commented Feb 11, 2020

__experimentalEditorSkeleton: Very recently merged as experimental, it seems it should continue as experimental. cc: @ockham

Fine with me! The overall idea here is to expose UI components from edit-post in order to allow 3rd parties to build customized editors; we'll probably add more of these in the future.

@nerrad
Copy link
Contributor

nerrad commented Feb 11, 2020

__experimentalCreateInterpolateElement: Hi @nerrad, should this mechanism be stabilized?

Sorry I missed the ping, I think this could be stabilized. The main reason for experimental was in case there might be need for argument changes but it appears how the function has been used to date is sufficient.

@chrisvanpatten
Copy link
Member

the corresponding selector __unstableIsEditorReady is marked as unstable (meaning it would not be desirable to ever stabilize), I think it should be blocked by a decision to remove or stabilize that unstable selector.

@aduth Are there additional tickets that explain this context? I had a problem today where __unstableIsEditorReady seemed like the ideal solution but I'm a little hesitant to use an unstable API if there's potential it will go away.

@aduth
Copy link
Member

aduth commented Feb 12, 2020

the corresponding selector __unstableIsEditorReady is marked as unstable (meaning it would not be desirable to ever stabilize), I think it should be blocked by a decision to remove or stabilize that unstable selector.

@aduth Are there additional tickets that explain this context? I had a problem today where __unstableIsEditorReady seemed like the ideal solution but I'm a little hesitant to use an unstable API if there's potential it will go away.

I don't know that there is an issue that encompasses it. It was introduced as part of #13088. The corresponding reducer has a bit more context:

* The editor is considered ready to be rendered once
* the post object is loaded properly and the initial blocks parsed.

If I recall, it was about avoiding to render any editor element until the post had finished loading (which could be an asynchronous request). It may have been marked as unstable because generally we should want to avoid these sorts of timeline milestones, in favor of deriving data as it happens to exist at any moment. In this case, "readiness" should probably be a derivation of the availability of a post / assigned blocks state, not a separate state value of its own. A selector may still make sense, but the reducer could be removed. At the time, I think it hadn't yet been fully developed to be able to do the sort of cross-store selectors that might have been needed to express this as a selector of 'editor' (example), which could have played a part in how this ended up being implemented.

tl;dr If it would be easy enough to express these conditions using other selectors, I expect it might be removed at some point (and for your requirements, you should consider if you could similarly try to derive conditions of readiness).

Aside: In retrospect, we should probably be much more clear about explicit "conditions for removal" of an unstable API, to avoid these situations of uncertainty.

Example:

export function __unstableIsLastBlockChangeIgnored( state ) {
// TODO: Removal Plan: Changes incurred by RECEIVE_BLOCKS should not be
// ignored if in-fact they result in a change in blocks state. The current
// need to ignore changes not a result of user interaction should be
// accounted for in the refactoring of reusable blocks as occurring within
// their own separate block editor / state (#7119).

@chrisvanpatten
Copy link
Member

Thanks @aduth. I’m sure I’m on an edge case but I’ve implemented a subscriber which is intended to watch for a specific meta attribute as soon as possibly, but strangely the getEditedPostAttribute selector itself is returned as undefined and seems to be causing some generally funky issues that were easily solved by using the isEditorReady selector. I’ll have to play around to see if I can find an alternative, and I appreciate the context!

@gziolo
Copy link
Member

gziolo commented Feb 14, 2020

I see that the list was updated, a few more comments:

__experimentalText: Recently merged as experimental, it seems soon to unmark. cc: @jameslnewell, @gziolo

There is no usage in Gutenberg of __experimentalText component so let's leave it as experimental.

__experimentalBlockVariationPicker: Hi @gziolo, the block variations is a feature of 5.4, should this component be stable?

It's not ready for prime time. Let's leave it as experimental.

__experimentalBlockSettingsMenuPluginsExtension

I opened #20233 where I promote it to the stable API as BlockSettingsMenuGroup - I'm not 100% sold on the name but it's enough to start the discussion. I still need to work on documentation.

@aduth
Copy link
Member

aduth commented Feb 27, 2020

__experimentalCreateInterpolateElement: Hi @nerrad, should this mechanism be stabilized?

Sorry I missed the ping, I think this could be stabilized. The main reason for experimental was in case there might be need for argument changes but it appears how the function has been used to date is sufficient.

While I think the function itself is reasonably stable, I'd want to draw some attention to the conversation at #19210 (comment), where some possible implementation of a custom React i18n module (if deemed reasonable) might be a better home for the function than in @wordpress/element.

@nerrad
Copy link
Contributor

nerrad commented Feb 27, 2020

While I think the function itself is reasonably stable, I'd want to draw some attention to the conversation at #19210 (comment), where some possible implementation of a custom React i18n module (if deemed reasonable) might be a better home for the function than in @wordpress/element.

I added some feedback there. To summarize, I don't think createInterpolateElement is only for i18n use (that's just currently the best and most prevalent use case). It could be consumed by any i18n package as needed.

@noisysocks
Copy link
Member

@noisysocks Reusable block related actions have been experimental for a year, I think with FSE coming stabilizing in 5.4 is not a good bet. Should we keep experimental in 5.4 and stabilize for 5.5 given the FSE efforts will be more advanced?

If anything they probably should be marked __unstable as, long term, I think these functions are best removed in lieu of getEntityRecord() and friends. I think taking no action is the correct thing to do here.

@jorgefilipecosta
Copy link
Member Author

Closing this issue as the audit is finished. I will try to follow up with some PR's based on these findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Type] Project Management Meta-issues related to project management of Gutenberg
Projects
No open projects
Development

No branches or pull requests