Open haines opened 7 months ago
Hi @haines I dug into this a bit and I was able to recreate as you mentioned. The issue seems to be the two different module/moduleResolution
combinations. If I switch both app
and lib
to use NodeNext/NodeNext
or if I switch both to use ESNext/Bundler
, it works fine. I'm not entirely sure why TypeScript is resolving the CJS export path however.
I know you linked a TypeScript issue above, but I'm not 100% sure it's the same problem. Would you be willing to file an issue on TypeScript's repo and post your repro there to see if they have any feedback or opinion on what's happening? If/when they reply, we can go from there on what the best approach is.
Hi @smaye81, I think you're right, this is not due to that TypeScript bug.
It's because of the node
export condition in package.json.
I used patch-package
to double-check exactly which imports were resolved and I was slightly surprised by the answer.
https://github.com/haines/buf-cjs-esm-ts/tree/check-which-types
==> ESNext/Bundler
loaded CJS
loaded proxy
loaded ESM types
==> NodeNext
loaded CJS
loaded proxy
loaded proxy types
So, at runtime, Node loads the proxy
module as specified here, and thus ultimately loads the CJS code.
TypeScript, however, ignores the node
condition in ESNext/Bundler
mode and loads the ESM types by following the import
condition here.
In NodeNext
mode, it follows the node
condition and thus loads the proxy
types (which just re-exports the CJS types). This is why the type mismatch occurs in a mixed project.
Perhaps the proxy should be re-exporting the ESM types instead?
Although, why is the node
condition necessary at all? I don't really want to load the CJS code in a modern ESM-supporting version of Node.js.
We are actually going to be fixing this in a future release of Protobuf-ES (see issue https://github.com/bufbuild/protobuf-es/issues/713). The plan will be to remove the node
condition and the module
condition entirely. However, when investigating with your repro, I also tested with this change in place and basically saw the same behavior. I still got the TypeError
that you noted above.
The one difference though was that if I flip-flopped your settings (app
using NodeNext/NodeNext
and lib
using ESNext/Bundler
), the build process succeeded if I tested with this change in place. If I flip-flop these settings and test with the current release of Protobuf-ES, I see the same TypeError
, only with the module format flip-flopped (CJS is not assignable to ESM).
This is where things get murky. I really have no idea how TypeScript is resolving these under the various circumstances. That's why it may be worth posting something on their repo to see if there's some fundamental issue with our setup or if this actually is a bug in their resolution logic.
We are actually going to be fixing this in a future release of Protobuf-ES (see issue #713).
Nice, makes sense.
However, when investigating with your repro, I also tested with this change in place and basically saw the same behavior.
Likewise. I have boiled it down to a fairly minimal reproduction (https://github.com/haines/typescript-dual-package-esnext-bundler-nodenext); it doesn't seem to be anything specific to your setup. I've raised https://github.com/microsoft/TypeScript/issues/57553.
Maybe mixing module
/moduleResolution
settings is a bad idea but it seems reasonable to me for our setup and isn't obviously verboten in the TS docs 🤷♂️ Hopefully someone will have some insight over there... this whole dual-package / ESM / CJS thing is a nightmare 😢
Great! Nice work on the repro. It looks like they agree it's probably a bug. Will be curious to hear what's going on.
I believe
@bufbuild/protobuf
(and the other@bufbuild
/@connectrpc
packages, which seem to have a similar setup) are affected by https://github.com/microsoft/TypeScript/issues/50466 - when compiled withNodeNext
module resolution, TypeScript resolves the CJS type definitions, not the ESM ones.I discovered this because it actually causes type errors in a monorepo build where an app with
Bundler
module resolution consumes generated messages from a library withNodeNext
resolution. Even though both projects should be ESM, the library refers to the CJS types for imports from@bufbuild/protobuf
.See https://github.com/haines/buf-cjs-esm-ts for a minimal repro. It produces incompatible type errors between
and
For example,