chartjs / chartjs/chartjs-plugin-zoom
Issues with TypeScript types
- Dominant language
- JavaScript
- Stars
- 623
- Forks
- 331
- Avg merge
- 7h 5m
- Merged PRs (30d)
- 7
Description
We're having some issues with the type definitions for chartjs-plugin-zoom. [Are the Types Wrong](https://arethetypeswrong.github.io/?p=chartjs-plugin-zoom%402.2.0) is reporting the following issues:
> | "chartjs-plugin-zoom"||
> |-- | --
> | node10 | ❗️ Incorrect default export
> | node16 (from CJS) | ❗️ Incorrect default export
> | node16 (from ESM) | ❗️ Incorrect default export🕵️ Named exports
> | bundler | ❗️ Incorrect default export
This is causing problems for my project when I set my project's tsconfig.json to `"module": "nodenext", "moduleResolution": "nodenext"`. For example:
```ts
import type ZoomPlugin from 'chartjs-plugin-zoom';
const scale = makeMyCustomScale();
function registerZoomFunctions(zoom: ZoomPlugin) {
zoom.zoomFunctions[scale.id] = () => false;
}
```
> error TS2339: Property 'zoomRectFunctions' does not exist on type 'typeof import("/Users/josh/src/app/node_modules/chartjs-plugin-zoom/types/index")'.
While investigating this, I also saw that the TypeScript port in `master` doesn't have functioning types at all: it specifies `dist/index.d.ts`, but that file doesn't exist. As I understand it, the correct solution would be to use rollup-plugin-dts or similar to combine the individual .d.ts files into a single .d.ts file under `dist` (matching the single `.js` file under dist) and to create separate .d.ts files for the CJS and ESM builds.
As I understand it, the problem is because a "true" CommonJS default export (as Rollup does [by default](https://rollupjs.org/configuration-options/#output-exports) when it can, and as represented in a TypeScript .d.ts by `export = ZoomPlugin`) is different than a CJS-ESM-interop default export (which is something like `exports.default = ZoomPlugin; Object.defineProperty(exports, '__esModule', { value: true });` and is represented by a TypeScript .d.ts as `export default ZoomPlugin`). I haven't found a good solution to this; I wonder if the simplest fix is to change Rollup to use named exports for its CJS build (and maybe even separate the CJS and UMD builds, as Chart.js itself does, to make this a little more explicit).
See https://github.com/chartjs/chartjs-plugin-annotation/pull/978 for where I'm working on a similar set of changes for chartjs-plugin-annotation. If you'd like for me to do something similar here, please let me know.
Contributor guide
Assessment
This issue has not been assessed yet.