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

useInstanceId: Fix useMemo hook dependencies #49979

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Conversation

Mamaduka
Copy link
Member

What?

PR fixes useMemo dependencies for the useInstanceId hook.

Why?

To avoid returning the stale values when preferredId or prefix change during render.

Testing Instructions

An example plugin for testing.

const { createElement: el, useState } = wp.element;
const { useSelect } = wp.data;
const { CheckboxControl } = wp.components;
const { useInstanceId } = wp.compose;
const registerPlugin = wp.plugins.registerPlugin;
const PluginDocumentSettingPanel = wp.editPost.PluginDocumentSettingPanel;

function MyDocumentSettingPlugin() {
    const [ isChecked, setChecked ] = useState( false );
	const id = useInstanceId( MyDocumentSettingPlugin, 'instance-prefix', isChecked ? 'custom-id' : undefined )

	return el(
		PluginDocumentSettingPanel,
		{
			className: 'test-checkbox',
			title: 'Test Checkbox',
		},
		el( CheckboxControl, {
            // id: 'test-foo', 
            checked: isChecked,
            label: 'Use custom Id',
            onChange: (value) => setChecked(value),
        } ),
		el( 'p', {}, id )
	);
}

registerPlugin( 'test-checkbox', {
	render: MyDocumentSettingPlugin,
} );

Screenshots or screencast

Before

CleanShot.2023-04-21.at.10.00.00.mp4

After

CleanShot.2023-04-21.at.10.00.41.mp4

@Mamaduka Mamaduka self-assigned this Apr 21, 2023
@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Package] Compose /packages/compose labels Apr 21, 2023
@Mamaduka Mamaduka requested review from tyxla and removed request for ajitbohra April 21, 2023 06:06
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Nice catch!

@github-actions
Copy link

Flaky tests detected in bdc91e9.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4761964063
📝 Reported issues:

@Mamaduka Mamaduka merged commit 92c6225 into trunk Apr 21, 2023
@Mamaduka Mamaduka deleted the fix/use-instance-id-deps branch April 21, 2023 06:45
@github-actions github-actions bot added this to the Gutenberg 15.7 milestone Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Compose /packages/compose [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants