Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript typings pollute global types by including lib.dom.d.ts #1656

Closed
piousdeer opened this issue Aug 26, 2020 · 3 comments · Fixed by #2174
Closed

TypeScript typings pollute global types by including lib.dom.d.ts #1656

piousdeer opened this issue Aug 26, 2020 · 3 comments · Fixed by #2174

Comments

@piousdeer
Copy link

Issue or Feature

types/index.d.ts contains the following line:

/// <reference lib="dom" />

It references lib.dom.d.ts, which usually isn't included in Node.js projects. This makes TypeScript think document, requestAnimationFrame and other browsers-only APIs are available in the global namespace.

Steps to Reproduce

mkdir canvas-bug
cd canvas-bug
npm init --yes
npm install --save-dev typescript @types/node
echo "document.write('Hello')" > index.ts
npx tsc --lib es2020 index.ts

As expected, the code will not compile because document doesn't exist in Node.js.

npm install canvas
echo "import { Canvas } from 'canvas'; document.write('Hello');" > index.ts
npx tsc --lib es2020 index.ts

The code will now compile despite being invalid.

Your Environment

  • Version of node-canvas (output of npm list canvas or yarn list canvas): 2.6.1
  • Environment (e.g. node 4.2.0 on Mac OS X 10.8): node 14.8.0 on Windows 10
@zbjornson
Copy link
Collaborator

Good point... any chance you know of a solution, aside from copying and pasting the relevant definitions into our typings file?

Copying them might not be a bad idea anyway. There are a few subtle errors in the current definitions because we don't 100% match the HTML Canvas spec.

@piousdeer
Copy link
Author

I'm not aware of a solution that wouldn't involve copying and pasting the official definitions. Like you said, doing that is probably not a bad idea after all.

@chearon
Copy link
Collaborator

chearon commented Dec 14, 2022

I just ran into this. It's super unfortunate that TypeScript works this way, as it can be really bad for Node or Node-and-browser codebases.

I've been skeptical before that node-canvas should use the DOM types at all. The types should match what we support perfectly, not the upstream types which might add features we don't support yet. If people want to draw to a context that's compatible with both DOM and node-canvas, they can accept an intersection of the two types.

chearon added a commit that referenced this issue Dec 14, 2022
chearon added a commit that referenced this issue Dec 18, 2022
zbjornson pushed a commit that referenced this issue Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants