`cloudflareDevProxyVitePlugin` returns error type under `NodeNext` module resolution

aaronadamsCA commented 1 month ago

Reproduction

System Info

System:
    OS: Linux 6.5 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    CPU: (4) x64 AMD EPYC 7763 64-Core Processor
    Memory: 8.68 GB / 15.61 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 20.16.0 - /usr/local/share/nvm/versions/node/v20.16.0/bin/node
    Yarn: 1.22.22 - /usr/bin/yarn
    npm: 10.8.1 - /usr/local/share/nvm/versions/node/v20.16.0/bin/npm
    pnpm: 9.6.0 - /usr/local/share/nvm/versions/node/v20.16.0/bin/pnpm

Used Package Manager

pnpm

Expected Behavior

Switching TypeScript to Node16 or NodeNext module resolution should not affect the return type of cloudflareDevProxyVitePlugin.

Actual Behavior

The Plugin type returned by cloudflareDevProxyVitePlugin() doesn't exist.

What I think is happening:

@remix-run/dev is a CommonJS package.
Vite v5 ships empty CommonJS types.
With Node16 module resolution, TypeScript transitively resolves Vite's CJS types, where Plugin doesn't exist.

Thankfully it's easy to work around.

I'm not sure if this would get better or worse once Vite v6 goes ESM-only, but hopefully React Router v7 can ship an ESM Vite plugin before then so we won't need to find out!

pcattori commented 1 month ago

We require Node v18+ across all of our packages (e.g. https://github.com/remix-run/remix/blob/main/packages/remix-react/package.json#L50), so I don't think supporting Node16 resolution makes sense for us. Closing, but if you think there's a reason to support Node16 resolution on Node 18+, let me know!

aaronadamsCA commented 1 month ago

These module resolution modes are for all modern Node.js versions, not for old ones.

Per the TypeScript docs:

node16, nodenext

These modes reflect the module resolution behavior of Node.js v12 and later. (node16 and nodenext are currently identical, but if Node.js makes significant changes to its module system in the future, node16 will be frozen while nodenext will be updated to reflect the new behavior.)

aaronadamsCA commented 1 month ago

For context, moduleResolution: "NodeNext" is compatible with, but stricter than, moduleResolution: "Bundler". Its more accurate type resolution behaviour has revealed all kinds of type import problems in the ecosystem, like packages using the same definition file for ESM and CommonJS entry points (which was a no-no from the start).

Honestly, Vite probably screwed up here by shipping a CommonJS entry point with no types. There are much better ways to deprecate an entry point!

But I think you can fix this with a TypeScript 5.3 feature that should let you import Vite's ESM types from your Vite plugin's CommonJS package:

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-3.html#stable-support-resolution-mode-in-import-types

remix-run / remix