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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS compilation since Async feature #82

Closed
Ricard opened this issue Jul 9, 2018 · 9 comments
Closed

TS compilation since Async feature #82

Ricard opened this issue Jul 9, 2018 · 9 comments

Comments

@Ricard
Copy link

Ricard commented Jul 9, 2018

Hello @pshihn, thank you for this great contribution 馃懐 馃憦

Since version 2.2.0, which introduce async methods, I'm not being able to build the library inside angular 6 app.
I do not provide OS and versions information because the issue reside on types of RoughCanvasAsync (promise of...) not matching the implemented class RoughCanvas.

Here you could see an evidence Build Status

@dispix
Copy link

dispix commented Jul 12, 2018

I have issues with the typing too. I wanted to test the library in a react + typescript environment and got this behavior:

  • In this first image, we see that the app renders fine but typescript triggers a type error:
    image
  • In this second image, we see that this time the typing is correctly applied but the application fails:
    image

Link to the sandbox

@pshihn
Copy link
Collaborator

pshihn commented Jul 14, 2018

@Ricard I realize that RoughCanvasAsync and RoughSync do not have the same return type
I have been using // @ts-ignore in the build which i realize is just a hack.
I did not want to break the pre-typescript version of the library by renaming the methods.

One option is that the return type is Drawable | Promise but that will add an extra check to the lib user.

Let me think about it a bit.

You could also start using the pre-built version of the library as well to get around it temporarily

@pshihn
Copy link
Collaborator

pshihn commented Jul 14, 2018

@dispix not sure what is happening in your case. I think it is picking up the wrong location of the lib/type defs. Maybe your tsconfig is not picking it up?

@pshihn
Copy link
Collaborator

pshihn commented Jul 14, 2018

@Ricard I have cleaned up the interfaces: #84 should fix this

@pshihn
Copy link
Collaborator

pshihn commented Jul 14, 2018

Available in v2.2.5

@dispix
Copy link

dispix commented Jul 20, 2018

@pshihn you can see the configuration in the sandbox, there's nothing special there and it works for other libraries so I have no idea where it comes from. I thought this was related to this issue but if you want I can create a new one.

@pshihn
Copy link
Collaborator

pshihn commented Jul 20, 2018

@dispix When I go to your url https://xoz411z0mq.codesandbox.io/ it loads fine and draws a box

Also, the error you're getting is that canvas is not defined in rough, where as type types clearly has the definition: https://github.com/pshihn/rough/blob/master/bin/rough.d.ts

My theory is that in the sandbox environment, either typings file is not being picked up, or when you import, it is importing the wrong type of module. There's a UMD version in the dist folder and there's the es6 module in the bin folder.

@pshihn
Copy link
Collaborator

pshihn commented Jul 20, 2018

It seems like codesandbox.io is not paying attention to the types property in package.json.

@pshihn
Copy link
Collaborator

pshihn commented Jul 20, 2018

Alright.. A couple of things:
Since module being loading typescript is not the commonjs module, https://www.typescriptlang.org/docs/handbook/modules.html says that import should be like this:

import rough from 'roughjs';

But it doesn't work properly on codesandbox.

Here's the working example based on your code that works on stackblitz:
https://stackblitz.com/edit/react-ts-mofibf

Hope this helps, @dispix

@pshihn pshihn closed this as completed Aug 3, 2018
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

No branches or pull requests

3 participants