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

Code quality: Find a flexible way to use stores that aren't always available #32842

Open
gziolo opened this issue Jun 21, 2021 · 4 comments
Open
Labels
[Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality

Comments

@gziolo
Copy link
Member

gziolo commented Jun 21, 2021

Description

We replaced hardcoded store keys with imported store definitions with #27088 and all related PRs. However, there are still several places where we have hardcoded store names, but they require some architectural changes because the stores used in the code are present conditionally depending on the execution context.

There is a group of items to look at raised for the code used with native mobile apps covered in #27751 issue.

In the web part of the universe, we still have two places that need more work:

Discovered in #32801:

// FIXME: @wordpress/block-library should not depend on @wordpress/editor.
// Blocks can be loaded into a *non-post* block editor.
// eslint-disable-next-line @wordpress/data-no-store-string-literals
const editorSelectors = select( 'core/editor' );
if ( editorSelectors ) {

Discovered in #32537

// FIXME: @wordpress/server-side-render should not depend on @wordpress/editor.
// It is used by blocks that can be loaded into a *non-post* block editor.
// eslint-disable-next-line @wordpress/data-no-store-string-literals
const coreEditorSelect = select( 'core/editor' );
if ( coreEditorSelect ) {

There are more places that require a fix as updated in #32866. It's related to the following core blocks:

  • Table of Contents
  • Template Part

My initial thought is that we should seek a way to consuming the data through block context. Although, it isn't clear how that could look in practice. I would start exploration with learning what shared data is available in PHP like the $post object or all the information that helps to decide what type of posts should be displayed on a given page. At least for the two cases referenced in this issue, the only data that is necessary can be retrieved from the currently edited $post object.

@youknowriad
Copy link
Contributor

A few thoughts

  • It feels fine to actually rely on the presence of a store or not, in some cases, the cases where a block may change the behavior a bit if it's in a post editor...

  • Some blocks seem to be actually "post editor" specific and may not make sense elsewhere (table of contents) and in this situation, the block need to be updated somehow to not render anything when it's outside "post content" (for instance in a template, widget editor), in some situations, it might be good to not register the block at all.

  • Template Parts: it seems it needs the available areas, could these areas be considered a block-editor setting and move there? Do they make sense in a WP agnostic context...

I think the main important thing here is to consider the current usage one by one and check the "meaning" of the dependency and check its conceptual impact.

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Jun 21, 2021

Template Parts: it seems it needs the available areas, could these areas be considered a block-editor setting and move there? Do they make sense in a WP agnostic context...

Another thought, their block variations are now registered server side with (i think) all the information necessary for this. We could probably replace this dependency with something like getBlockVariations from the core/blocks store. We may be able to get rid of that editor store settings prop altogether. 🤔

Edit - ah but that server side variation registration doesn't work with all WP versions we support, so the areas would be limited to the variation fallbacks defined in javascript. This may be acceptable?

@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Apr 1, 2022

Copying my pondering from #39987: could dynamic import be useful here?

// This will either be the store or null.
const editorStore = await import('@wordpress/editor')
	.then(result => result.store)
	.catch(_err => null)

@youknowriad With regard to the Table of Contents block, one use-case I wanted to eventually cover was adding a ToC to your site's Post template, without having it in the post content itself. This was back when the block relied heavily on dynamic rendering, though. I'm not sure if such a thing is still feasible given the current static implementation of #29739, though recent discussions may be moving the block back in a dynamic direction. The main issue to tackle would be getting the blocks inside the Post Content... from outside of the Post Content.

So I'm not sure if that particular use case will truly end up being post-editor-only or not. If it does, the solution would be to move it to the edit-post package, right? The Legacy Widget block is already in the edit-widgets package, for example.

@youknowriad
Copy link
Contributor

The main issue to tackle would be getting the blocks inside the Post Content... from outside of the Post Content.

This should be possible (you can get the blocks like we do in the "post content" block) and it should work (at least in the "edit" of the block. and yeah, it should be rendered dynamically if that's what we want to do.

Copying my pondering from #39987: could dynamic import be useful here?

I don't think there needs to be a dependency to "editor" to solve the issue here (useEntityProp is from core-data). Now to answer your question specifically, we can't use dynamic imports yet, and dynamic imports even if possible don't change the "logic", meaning it's still a depedency and not a conditional one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

4 participants