types: Export types in a way compatible with TypeScript "moduleResolution": "NodeNext"#200
types: Export types in a way compatible with TypeScript "moduleResolution": "NodeNext"#200brettwillis wants to merge 3 commits intofastify:masterfrom brettwillis:fix-typings-for-esm
"moduleResolution": "NodeNext"#200Conversation
|
@fastify/typescript could you take a look? |
|
Oh. This could be a problem, and we should find a solution. @brettwillis, we export default (and map it in JS code) to support every environment, bundler, and compiler out there. We call it the "famous triplet" because it allows us to make Fastify modules compatible with ESM, faux ESM, TS, and CJS. Changing our types to "namespace" will make them usable only with |
|
@fox1t You may want to take a look at fastify/fastify-cookie#184 which is the similar approach. |
|
@climba03003 I am starting to think that the era of the "famous triplet" is over. If so, we should update all of our types accordingly. As far as I understood from your link the issue is |
Yes. and No. It still supported somehow, but it has more problem pop-up.
Yes
Yes, this combination is the major problem. |
|
After further discussion with @climba03003 and basic repro on my machine, I would not like to make these types CJS (aka using |
|
P.S. I still want to see a repro repo here because I am unsure about the fix. |
Co-authored-by: KaKa <climba03003@gmail.com>
|
Can we have a test for the last change? |
|
Sorry for the delay, will put together the repro tomorrow. |
|
Hi all, please see this branch in my fork https://github.com/brettwillis/fastify-plugin/tree/fix-typings-repros, and find the directories named:
The test setup looks like this import fp1, { PluginOptions } from 'fastify-plugin';
import { default as fp2, PluginMetadata } from 'fastify-plugin';
import * as fp3 from 'fastify-plugin';
console.log(typeof fp1); // function
console.log(typeof fp1.default); // function
console.log(typeof fp2); // function
console.log(typeof fp2.default); // function
console.log(typeof fp3); // object
console.log(typeof fp3.default); // function
fp1(async () => {});
fp2(async () => {});
// esModuleInterop has no influence here, always type error and runtime error, so the typing is correct, this is consistent with old
// @ts-expect-error
fp3(async () => {}); // runtime TypeError: fp3 is not a functionIn summary
Please double-check my work and let me know your thoughts. |
|
@climba03003 @fastify/typescript I completely defer to you on this one. |
|
@climba03003 apologies for the delay, it's pretty busy on my end. I've added tests for the the different import styles, but I'm unsure how to go about invoking with different |
This is a typing issue only, not a runtime isse. Since TypeScript
4.8.3, when using"moduleResolution": "NodeNext"(or"Node16") for an ESM project, TypeScript will now report an error when using the code below:My own understanding is limited, but there has been a lot of discussion, for example microsoft/TypeScript#48845. Basically I think it is because the typings descibe the module as an ES module with a default export (which would be correct if the module was an ES module), but it is actually a CJS module exporing the
pluginfunction with adefaultproperty...Either way, I've tested this PR in an ESM project with
"moduleResolution":"Node"and"moduleResolution":"NodeNext"(or"Node16") and it compiles successfully for both.Checklist
npm run testandnpm run benchmarktests and/or benchmarks are includeddocumentation is changed or addedand the Code of conduct