Closed yutak23 closed 1 year ago
@yutak23 1st of all: thanks a lot for your effort and patience
I'm not completely sure about the approach. Although it seems to solve the typings problem, IMO it increases the cost of maintenance. At this point, wouldn't it make sense to port the project to TS?, so we also get rid of having to synchronise the TS definitions, WDYT?
I'd like to get more feedback on this from other maintainers
@yutak23 1st of all: thanks a lot for your effort and patience
I'm not completely sure about the approach. Although it seems to solve the typings problem, IMO it increases the cost of maintenance. At this point, wouldn't it make sense to port the project to TS?, so we also get rid of having to synchronise the TS definitions, WDYT?
I'd like to get more feedback on this from other maintainers
@mindhells
Thank you for your comment.
I agree that it would require maintenance of both JavaScript and TypeScript types, and in that sense would increase maintenance costs.
I agree with you on the move to TypeScript.
I have changed my PR content and raised another PR. This one is closed.
This ought to fix https://github.com/softonic/axios-retry/issues/228
ESM seems incompatible in CommonJS PJs with default export (see https://github.com/microsoft/TypeScript/issues/52086). I think there are two solutions to make TypeScript types compatible with both CommonJS and ESM.
Case
1
, I would make a copy of the currentindex.d.ts
, rename the file toindex.d.mts
, and change package.json as follows.However, I think this is redundant and low-maintainability because we have to manage two type definitions, so I tried to modify it in the second way(Case
2
).Explanation of this modification
By setting
export =
, both CommonJS and ESM are supported. I do not expect any impact on existing CommonJS implementations of TypeScript developers. Originally, when usingexport default
, as described in Default Exports, theesModuleInterop
option must be used, and testing has confirmed that it can be compiled with this setting. Also, testing confirms that the project can be compiled with the project in the ESM configuration.dtslint and index.test-d.ts I'm adding dtslint and type definition tests to this project to match the axios project setup. The settings in tsconfig.json and tslint.json for dtslint are copied almost directly from axios, so there should be no problems. I think it should be included because the addition of dtslint allows us to safely update type definitions. (More than half of the modifications to existing type definitions is based on dtslint checks.)
For this change, I refer to the following