Tooltip: Fix Ariakit tooltip store usage

[tyxla]

What?

This PR fixes how we use the Ariakit.useTooltipStore hook in the Tooltip component.

Why?

Previously, we were circumventing an ESLint error because the hook was conditionally called. I recall I suggested this workaround, so I'm glad I'm the one to suggest removing it now (see #57202 (comment)).

The workaround is no longer necessary since the hook is no longer called conditionally.

The motivation is that I'm fixing it to resolve a couple of ESLint errors that were raised by the React Compiler ESLint plugin in #61788

How?

Just using the hook directly.

Testing Instructions

Verify the static analysis check is still green.

Testing Instructions for Keyboard

None

Screenshots or screencast

None

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: diegohaz <hazdiego@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Flaky tests detected in 98fa4cc.
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/9189637481
📝 Reported issues:

• [Flaky Test] Clicking on inserter while on zoom-out should open the patterns tab on the inserter #61806 in /test/e2e/specs/site-editor/zoom-out.spec.js

[Mamaduka]

Thanks, @tyxla!

We also need to figure out how to handle warnings/errors for Ariakit's store hooks.

const compositeStore = useCompositeStore();
const activeId = compositeStore.useState( 'activeId' );

[tyxla]

We also need to figure out how to handle warnings/errors for Ariakit's store hooks.

const compositeStore = useCompositeStore();
const activeId = compositeStore.useState( 'activeId' );

Definitely. Might need some additional wrapper, in Ariakit or here, in the package. One simple solution I can think of is having a separate useCompositeStoreState( store ) hook that accepts a store argument and will return the internal state of that store based on that store argument. Might also need some name shuffling to prevent ESLint from being sad again.

[tyxla]

@diegohaz @DaniGuardiola is this a problem you've encountered and/or thought about already in Ariakit?

[diegohaz]

I'm not sure I'm aware of that problem. Could you elaborate?

[tyxla]

We're trying out the new React Compiler ESLint plugin in #61788

It will complain that hooks can't be just dynamically declared or assigned somewhere. So in code like @Mamaduka provided above:

const compositeStore = useCompositeStore();
const activeId = compositeStore.useState( 'activeId' );

it will complain because the useState hook reference can potentially change, and that's against the rules of hooks - more details here.

To solve this issue, useState must be recognized by the React Compiler as a stable, static reference. My conclusion is that we might need a different API for that (as I roughly suggested above), but I'm curious if you have any other ideas.

[Mamaduka]

@diegohaz, the following code triggers a warning/error when enabling the new React compiler ESLint rules.

[2] /home/runner/work/gutenberg/gutenberg/packages/components/src/alignment-matrix-control/index.tsx
[2]   71:19  error  Hooks must be the same function on every render, but this value may change over time to a different function. See https://react.dev/reference/rules/react-calls-components-and-hooks#dont-dynamically-use-hooks  react-compiler/react-compiler

gutenberg/packages/components/src/alignment-matrix-control/index.tsx

Lines 59 to 71 in 6e8c261

	const compositeStore = useCompositeStore( {
	defaultActiveId: getItemId( baseId, defaultValue ),
	activeId: getItemId( baseId, value ),
	setActiveId: ( nextActiveId ) => {
	const nextValue = getItemValue( baseId, nextActiveId );
	if ( nextValue ) {
	onChange?.( nextValue );
	}
	},
	rtl: isRTL(),
	} );
	const activeId = compositeStore.useState( 'activeId' );

[diegohaz]

I see. Ariakit has an experimental useStoreState function (currently exported by @ariakit/react-core/utils/store) that can be used like this:

const activeId = useStoreState(compositeStore, 'activeId');

This is essentially what compositeStore.useState does internally. There are no plans to promote this API to @ariakit/react soon, but it might happen if this is a general issue.

[tyxla]

There are no plans to promote this API to @ariakit/react soon, but it might happen if this is a general issue.

I think that might be the case. Thanks for considering it. Also, cc @DaniGuardiola who's working on the Ariakit upgrade in #60992

[DaniGuardiola]

@tyxla I'm not sure this PR is doing anything other than trick eslint into thinking there's no hook to analyze. The React linting tule is just too dumb to identify namespaces hooks as such, but it is still a hook and it should still respect the rules of hooks that eslint complained about.

Separately, we should wrap these hooks into ones that throw if the context is not present, so that no conditional logic is necessary (this is also what we discussed regarding Tabs).

Edit: I think I misunderstood, will check again once I'm back from lunch.