adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.21k stars 1.07k forks source link

Published @react-types broken for Menu component #5818

Open jaens opened 5 months ago

jaens commented 5 months ago

Provide a general summary of the issue here

The react-aria-components published version 1.0.1 Menu implementation (which I assume derives from @react-aria/menu) is out of sync with the published types in @react-types/menu.

๐Ÿค” Expected Behavior?

autoFocus works.

๐Ÿ˜ฏ Current Behavior

Published component uses the autoFocus property to set initial focus. @react-types/menu version 3.9.6 does not declare this property... TypeScript fails to compile. If you override the TS error it works.

Note that @react-aria/menu is version 3.12.0, which I assume r-a-c uses.

The type as currently declared is:

export interface MenuProps<T> extends CollectionBase<T>, MultipleSelection {
  /** Where the focus should be set. */
  isOpen?: boolean | FocusStrategy,
  ...

Looks like isOpen got renamed to autoFocus at some point.

๐Ÿ’ Possible Solution

Re-publish @react-types/menu?

Seems like having separate @react-types packages is quite error prone, it would make more sense to provide types with the main packages (ie. @react-aria/menu), like 90% of other TypeScript projects?

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

Install latest react-aria-components and @react-aria/menu.

Add following TS code somewhere:

import { Menu } from "react-aria-components";

<Menu autoFocus="first" />

Version

react-aria-components 1.0.1

What browsers are you seeing the problem on?

Other

If other, please specify.

No response

What operating system are you using?

N/A

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

jaens commented 5 months ago

Workaround for anyone running into the same problem:

Install the nightly tag of react-aria-components and react-aria.

Do not depend on any @react-aria/* packages. If you need to do that, use pnpm and add the following to .npmrc:

public-hoist-pattern[]=@react-aria/*

This will guarantee compatible versions of all packages... ugh.

Real semver of a monorepo is hard and error-prone, it would seem easier to just lock all inter-package deps at an exact version...

snowystinger commented 5 months ago

Can you reproduce this in a codesandbox or stackblitz? I'm unable to https://stackblitz.com/edit/react-ts-8glnm3?file=package.json,App.tsx

You may be facing duplicate packages, if you clear out your lockfile it will likely resolve. See https://github.com/adobe/react-spectrum/wiki/Frequently-Asked-Questions-(FAQs)#why-are-there-errors-after-upgrading-a-react-spectrum-package for more explanation.

We've discussed locking before, however, we do have real life use cases where people need to substitute a specific version of a given package. We may update this down the road.

jaens commented 5 months ago

StackBlitz does not seem to do typechecking? I added whatever="string" to the <Menu> tag and everything was okey-dokey. I do not think it is possible to reproduce type errors there if it can not catch something that simple :smiling_face_with_tear:

snowystinger commented 5 months ago

Interesting, I wonder what is going on there. It has correct types for the MenuTrigger. Will need more time to look into it.