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

@headlessui/react is not compatible with TypeScript 4.7 NodeNext module resolution. #1699

Closed
scott-lc opened this issue Jul 19, 2022 · 8 comments · Fixed by #1721
Closed
Assignees

Comments

@scott-lc
Copy link

@headlessui/react

What version of that package are you using?

v1.6.6

What browser are you using?

Not a browser issue. Rather this is a TypeScript compiler issue. Environment is:

  • macOS 11.4
  • node v18.3.0
  • npm v8.14.0
  • TypeScript 4.7.4 using NodeNext module resolution in the tsconfig.json file.

Reproduction URL

https://github.com/scott-lc/headlessui-nodenext

npm install
npm run build:esnext # works fine, like expected
npm run build:nodenext # big time fail

Describe your issue

TypeScript 4.7 introduced a new ESM-compliant module resolution specifier called NodeNext (details here).

One fallout of this is that when compiling TypeScript code with nodenext, the compiler expects that code that "claims" to be ESM compliant is actually ESM compliant. How does code claim it is ESM compliant? From TypeScript 4.7's nodenext module resolution algorithm perspective, the presence of "type": "module" in the dependency's package.json file indicates that the dependency should be interpreted as ESM.

As described here, when a dependency is marked with "type":"module", it is interpreted as an ES Module and "a few different rules come into play". Specifically:

Relative import paths need full extensions (we have to write import "./foo.js" instead of import "./foo").

The underlying issue is that @headless-ui/react is marked as being ESM compliant (`"type":"module"), but it really isn't. In order to be ESM compliant, all imports must use a full extension. This is nothing new and has been true since ESM support was marked as stable sometime back in Node 14.

What has changed is that the TypeScript 4.6 esnext/node module resolver did not care. It happily consumed code that was missing extensions or made assumptions about auto-resolving index.xx files when importing a directory. In fact, Node itself has a flag to resolve modules "the old way".

However, when using TypeScript 4.7 nodenext module resolution algorithm and things fail spectacularly:

src/index.tsx:3:10 - error TS2305: Module '"@headlessui/react"' has no exported member 'Listbox'.

import { Listbox } from "@headlessui/react";

node_modules/@headlessui/react/dist/index.d.ts:1:15 - error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './components/combobox/combobox.js'?

1 export * from './components/combobox/combobox';

node_modules/@headlessui/react/dist/index.d.ts:2:15 - error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './components/dialog/dialog.js'?

2 export * from './components/dialog/dialog';

What does this mean for @headlessui/react? It really should be as straightforward as adding .js extensions to the typescript code as described here.

@scott-lc
Copy link
Author

The easier solution is to remove:

{  
  "type": "module"
}

from the @headlessui/xxx/package.json files. This would not require and changes to the TS files. The few plain JavaScript files that you are using would then need to be renamed as as:

foo.cjs => foo.js
foo.js => foo.mjs

@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

We already rewrote the imports for .js files, but we forgot to do that on the .d.ts files.

This should be fixed by #1721, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

When I try this version in your reproduction repo, it seems to not fail as spectacularly as before, however I do get this error now:

src/index.tsx:3:25 - error TS1471: Module '@headlessui/react' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

3 import { Listbox } from "@headlessui/react";
                          ~~~~~~~~~~~~~~~~~~~


Found 1 error in src/index.tsx:3

I'm debugging this issue right now to see if we have to make more changes to Headless UI itself or not.

@RobinMalfait
Copy link
Collaborator

import { StrictMode, type ReactElement } from "react";
import { createRoot } from "react-dom/client";
import React, { useState } from "react";

const people = [
  { id: 1, name: "Durward Reynolds", unavailable: false },
  { id: 2, name: "Kenton Towne", unavailable: false },
  { id: 3, name: "Therese Wunsch", unavailable: false },
  { id: 4, name: "Benedict Kessler", unavailable: true },
  { id: 5, name: "Katelyn Rohan", unavailable: false },
];

async function WHY() {
  const { Listbox } = await import("@headlessui/react");

  const App = (): ReactElement => {
    const [selectedPerson, setSelectedPerson] = useState(people[0]);

    return (
      <Listbox value={selectedPerson} onChange={setSelectedPerson}>
        <Listbox.Button>{selectedPerson.name}</Listbox.Button>
        <Listbox.Options>
          {people.map((person) => (
            <Listbox.Option
              key={person.id}
              value={person}
              disabled={person.unavailable}
            >
              {person.name}
            </Listbox.Option>
          ))}
        </Listbox.Options>
      </Listbox>
    );
  };

  createRoot(document.querySelector("#root")!).render(
    <StrictMode>
      <App />
    </StrictMode>
  );
}

WHY();

This works, but this is far from ideal of course.


I see people recommending to use esnext instead of nodenext for this 🙃 I'll keep digging, but if you immediately know what this means then feel free to comment here as well 😅.

@scott-lc
Copy link
Author

@RobinMalfait

Above you mentioned an error you were still seeing with the insiders release:

src/index.tsx:3:25 - error TS1471: Module '@headlessui/react' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

3 import { Listbox } from "@headlessui/react";

Did you fix it (not counting the await hack)? Everything compiles fine for me in both my upgraded example repo as well as my actual project.

@RobinMalfait
Copy link
Collaborator

Hey @scott-lc, so it is working for you?

Interesting, I'm on your repo and bumped Headless UI but it's not working or me in this case.

However, I tested it on existing projects and it works there jus fine. So if it is working for you than I'm calling this change a success 😅

Screenshot of my changes:

image

@scott-lc
Copy link
Author

@RobinMalfait - I have the exact same change and cannot re-create what you are seeing. I too call this change a success. Bravo and thanks.

  1. I know is is basically the software equivalent to "turn it off and on again", but can you delete your (package-|yarn).lock file the node_modules directory and re-install the dependencies from scratch?

  2. Try adding the following to package.json

{
  "type": "module"
}

although that should not matter.

@scott-lc
Copy link
Author

I did re-create it. My dependency was ^0.0.0-insiders.5bfa3da (with caret) so I must have picked up the wrong dependency.

Without the package.json "type":"module, I see the same compiler error you do.

@jerriclynsjohn
Copy link

Eventhough I was able to remove the error by installing the insider version, I got another error when i use Dialog.panel and it says

Property Panel does not exist

as mention in #1419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants