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.

Sign up for GitHub

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

Populate block context with inherited post type from template slug #65062

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Populate block context with inherited post type from template slug #65062

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9382b2a
Inherit post type from template slug
SantosGuillamot Sep 4, 2024
ac443f5
Move map outside of `useSelect`
SantosGuillamot Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 40 additions & 10 deletions packages/editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
import { useEffect, useLayoutEffect, useMemo } from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { EntityProvider, useEntityBlockEditor } from '@wordpress/core-data';
import {
EntityProvider,
useEntityBlockEditor,
store as coreStore,
} from '@wordpress/core-data';
import {
BlockEditorProvider,
BlockContextProvider,
Expand Down Expand Up @@ -48,7 +52,6 @@ const noop = () => {};
*/
const NON_CONTEXTUAL_POST_TYPES = [
'wp_block',
'wp_template',
'wp_navigation',
'wp_template_part',
];
Expand Down Expand Up @@ -161,31 +164,57 @@ export const ExperimentalEditorProvider = withRegistryProvider(
BlockEditorProviderComponent = ExperimentalBlockEditorProvider,
__unstableTemplate: template,
} ) => {
const { editorSettings, selection, isReady, mode } = useSelect(
( select ) => {
const { editorSettings, selection, isReady, mode, postTypes } =
useSelect( ( select ) => {
const {
getEditorSettings,
getEditorSelection,
getRenderingMode,
__unstableIsEditorReady,
} = select( editorStore );
const { getPostTypes } = select( coreStore );

return {
editorSettings: getEditorSettings(),
isReady: __unstableIsEditorReady(),
mode: getRenderingMode(),
selection: getEditorSelection(),
postTypes:
getPostTypes( { per_page: -1 } )?.map(
( entity ) => entity.slug
) || [],
Copy link
Member

Choose a reason for hiding this comment

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

You should avoid map/filter operations for returned values from mapSelect. They will always return new reference and force component to re-render even if derived value is the same.

P.S. You should see warning in console for similar cases in dev mode. See #53666.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info! I wasn't aware of that 🙂 I have just changed it in this commit.

Not related to this pull request, but do you think it could be an issue also here? I'm happy to create another PR to change that.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, @SantosGuillamot!

Not related to this pull request, but do you think it could be an issue also here? I'm happy to create another PR to change that.

That one is okay. It will pass a shallow equality check since the filteredPostTypes values will have the same reference. It's somewhat undocumented and a last-resort hack.

It usually depends on the case; check for a warning in the DevTools if you're unsure.

// ❌ will fail shallow equality since the `postType` property has a new reference on each call.
return {
   postTypes: types.filter( () => /*--/* );
}

// ✅ will pass shallow equality since returned object properties have the same object reference
return types.filter( () => /*--/* );

};
},
[]
);
}, [] );
const shouldRenderTemplate = !! template && mode !== 'post-only';
const rootLevelPost = shouldRenderTemplate ? template : post;
const defaultBlockContext = useMemo( () => {
const postContext =
const postContext = {};
// If it is a template, try to inherit the post type from the slug.
if ( post.type === 'wp_template' ) {
if ( ! post.is_custom ) {
const [ kind ] = post.slug.split( '-' );
switch ( kind ) {
case 'page':
postContext.postType = 'page';
break;
case 'single':
// Infer the post type from the slug.
const match = post.slug.match(
`^single-(${ postTypes.join( '|' ) })(?:-.+)?$`
);
if ( match ) {
postContext.postType = match[ 1 ];
}
break;
}
}
} else if (
! NON_CONTEXTUAL_POST_TYPES.includes( rootLevelPost.type ) ||
shouldRenderTemplate
? { postId: post.id, postType: post.type }
: {};
) {
postContext.postId = post.id;
postContext.postType = post.type;
}

return {
...postContext,
Expand All @@ -200,6 +229,7 @@ export const ExperimentalEditorProvider = withRegistryProvider(
post.type,
rootLevelPost.type,
rootLevelPost.slug,
postTypes,
] );
const { id, type } = rootLevelPost;
const blockEditorSettings = useBlockEditorSettings(
Expand Down
Loading