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

OffscreenCanvas types missing #1234

Closed
ronyeh opened this issue Nov 15, 2021 · 5 comments · Fixed by #1230
Closed

OffscreenCanvas types missing #1234

ronyeh opened this issue Nov 15, 2021 · 5 comments · Fixed by #1230

Comments

@ronyeh
Copy link
Collaborator

ronyeh commented Nov 15, 2021

@tommadams for advice.

After I npm installed the new vexflow 4 beta, I tried building a barebones TS project and I get these errors.

ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts
15:40-73
[tsl] ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts(15,41)
      TS2304: Cannot find name 'OffscreenCanvasRenderingContext2D'.

ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts
21:30-45
[tsl] ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts(21,31)
      TS2304: Cannot find name 'OffscreenCanvas'.

ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts
56:50-83
[tsl] ERROR in examples/ts/node_modules/vexflow/src/canvascontext.ts(56,51)
      TS2304: Cannot find name 'OffscreenCanvasRenderingContext2D'.

Is it because the @types/offscreencanvas is a devDependency? I guess I'll try moving it to regular dependencies to see if it changes anything.

@tommadams
Copy link
Contributor

Ah, my bad. Yes, it's probably because offscreencanvas is a dev dependency not a regular one.

I compile vexflow and my personal project separately, which is why I missed that.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 15, 2021

microsoft/types-publisher#81 (comment)

This might be relevant. I'll try moving it over to a regular dependency.

ronyeh added a commit to ronyeh/vexflow4 that referenced this issue Nov 16, 2021
@ronyeh ronyeh linked a pull request Nov 16, 2021 that will close this issue
@0xfe
Copy link
Owner

0xfe commented Nov 20, 2021

So I want to really preserve the "no dependency" aspect of VexFlow. I'm guessing the non-dev dependency here is temporary until it's broadly available in TS, and we can remove the dependency, right?

@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 21, 2021

I like the idea of "no (non-dev) dependencies". For this case, it could take a while for it to be included in TypeScript since it is enabled by default only in Chrome, Edge, and Opera. https://developer.mozilla.org/en-US/docs/Web/API/OffscreenCanvas/OffscreenCanvas#browser_compatibility

The good news... the entire dependency is a single 82-line type definition file: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/offscreencanvas/index.d.ts

Since it's MIT licensed, we could copy it directly into our repository (while maintaining attribution), and then remove it from our package.json. Benefit: no NPM dependency. However, if this d.ts file is ever updated, we'll have to manually replace our copy (assuming we are aware of the update).

Does this sound like a good approach? Should we just include the d.ts file in our repo?

@0xfe
Copy link
Owner

0xfe commented Nov 21, 2021

Thanks for digging in, Ron. Since this will eventually go away, this is fine with me. (Also type defs are just used during build time, so it's not a huge concern.) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants