downshift-js / downshift

🏎 A set of primitives to build simple, flexible, WAI-ARIA compliant React autocomplete, combobox or select dropdown components.
http://downshift-js.com/
MIT License
12.09k stars 929 forks source link

Preact build imports hooks from wrong module #1198

Open cmlenz opened 3 years ago

cmlenz commented 3 years ago

Relevant code or config

import {useSelect} from 'downshift/preact';
import {h} from 'preact';

export function Select({items}) {
  const {getItemProps, getMenuProps, getToggleButtonProps, isOpen, selectedItem} = useSelect({items});

  return (
    <div>
      <button type='button' {...getToggleButtonProps()}>
        {selectedItem}
      </button>
      <ul {...getMenuProps()}>
        {isOpen &&
          items.map((item, index) => (
            <li
              key={`${item}${index}`}
              {...getItemProps({item, index})}
            >
              {item}
            </li>
          ))}
      </ul>
    </div>
  );
}

What you did:

I tried using Downshift with Preact without the preact-compat layer. The above code is a somewhat simplified version of the DropdownSelect component from the documentation.

What happened:

First, the Webpack build warns that it can't resolve the useRef import:

WARNING in ./node_modules/downshift/preact/dist/downshift.esm.js 3256:24-30
"export 'useRef' was not found in 'preact'
 @ ./select.tsx

At runtime, I'm getting the following error in Chrome:

TypeError: Object(...) is not a function
    at useEnhancedReducer (downshift.esm.js:1788)
    at useControlledReducer (downshift.esm.js:1831)
    at useSelect (downshift.esm.js:2460)
    at d.Select [as constructor] (select.tsx:5)
    at d.M [as render] (preact.module.js:1)
    at $ (preact.module.js:1)
    at m (preact.module.js:1)
    at H (preact.module.js:1)
    at $ (preact.module.js:1)
    at m (preact.module.js:1)

Reproduction repository:

None yet

Problem description:

Investigating the issue, I found the following line in the pract/dist/downshift.esm.js file:

import { cloneElement, Component, useRef, useEffect, useCallback, useReducer, useMemo } from 'preact';

That seems incorrect, as Preact exposes the hook functions not from the preact module but from preact/hooks.

Suggested solution:

The build would need to be changed to import the hooks from preact/hooks instead of preact, so the line above would instead be:

import { cloneElement, Component } from 'preact';
import { useRef, useEffect, useCallback, useReducer, useMemo } from 'preact/hooks';
developit commented 3 years ago

One convenient way of addressing this would be to build the preact version with react aliased to a proxy module that combines these exports:

export * from 'preact';
export * from 'preact/hooks';

(slightly more complete version)

silviuaavram commented 3 years ago

I admit that when developing the hooks I did not think about preact much, and I have no knowledge of preact myself either.

However, I do want for Downshift to continue to support preact so please feel free to suggest fixes and improvements, with as much detail as possible, and create PRs. Thank you!

MilkyMike commented 3 years ago

We use preact in combination with downshift as well. @silviuaavram I think you can easily fix this issue by changing the import in /node_modules/downshift/preact/dist/downshift.esm.js from import { cloneElement, Component, useRef, useEffect, useLayoutEffect, useCallback, useReducer, useMemo } from 'preact'; to import { cloneElement, Component, useRef, useEffect, useLayoutEffect, useCallback, useReducer, useMemo } from 'preact/compat';

silviuaavram commented 3 years ago

@MilkyMike can you create a PR with this fix please?

MilkyMike commented 3 years ago

Hey @silviuaavram, while it is really easy for me to fix this issue in the dist folder after the build, it is way more difficult for me to do this before the build. I don't know your code, I don't know your build process and I am far from a js pro. From the time I have looked into your project I suspect it is some magic in the kcd-scripts that you are using to build but I am not sure at all. So I have to apoligize and hope someone else can do it for me.

evanfrawley commented 2 years ago

The imports for hooks are still coming from 'preact' instead of 'preact/compat' so this issue is still broken. Looks like it is an underlying issue with Kent C Dodd's build scripts. My guess is that this PR actually didn't solve the 'preact/compat' issue, or if it did solve back when it merged, it has regressed since then