chartjs / chartjs/chartjs-plugin-zoom

Issues with TypeScript types

Open
#941 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.