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

Support TypeScript #139

Closed
FrankBrullo opened this issue Nov 1, 2018 · 16 comments
Closed

Support TypeScript #139

FrankBrullo opened this issue Nov 1, 2018 · 16 comments

Comments

@FrankBrullo
Copy link

It seems that the library doesn't have the Typescript types defined.
I tried to do npm i @types/britecharts-react but the module doesn't exist.

I used react-scripts-ts to create my react test project.

Below the TypeScript error I get from the compiler when I import the library:

Could not find a declaration file for module 'britecharts-react'. 'test-app/node_modules/britecharts-react/dist/britecharts-react.min.js' implicitly has an 'any' type.
Try npm install @types/britecharts-react if it exists or add a new declaration (.d.ts) file containing declare module 'britecharts-react';

@Golodhros
Copy link
Collaborator

Hi @FrankBrullo, thanks for opening this issue.

Yes, we don't have TS types. Several people talked about it in https://github.com/eventbrite/britecharts/issues/223, but never came out of it yet.

We are open to PRs though.

@FrankBrullo
Copy link
Author

FrankBrullo commented Nov 8, 2018

Ok, I will do and let you know when the PR is ready.
Also, while I am looking at the Donut, I am not seeing implemented customMouseMove and more importantly customClick.
Any idea why?
How can I get the click event in React if they aren't defined as props?

Thanks.

@Golodhros
Copy link
Collaborator

That would be awesome @FrankBrullo!! Thanks a lot for doing this!!

True, those handlers are not set, but they should. We would need to add them there as proptypes and they should be ready to use!

@FrankBrullo
Copy link
Author

@Golodhros That would be great!
Do you have an ETA on when it will be fixed?
I need to include those events in my Typescript types for the Pie Chart before sending the PR to you.

Thanks,
Frank

@Golodhros
Copy link
Collaborator

I will try to go over it this weekend, but until we fix the release issue(hopefully next week) we won't be able to publish it.

@FrankBrullo
Copy link
Author

Let me know when you push your changes on Git so I can update my fork.
I hope to deliver the Typescript changes before your release so we will have everything in one shot.

@miglesiasEB
Copy link
Collaborator

Handlers Added!

@miglesiasEB
Copy link
Collaborator

Hi @FrankBrullo, the handlers are ready and the release pipeline is working again!

@dupski
Copy link

dupski commented Dec 4, 2018

+1 for TS support. Give me a shout if you need any assistance @FrankBrullo :)

@FrankBrullo
Copy link
Author

Hi @miglesiasEB sorry for the radio silence but I got some issues to continue working on the PR.
Now I have sorted out everything and I have restarted.
Is there a way I can get an invite to join your slack channel so I can ask you a couple of questions I have?

@dupski Thank you for the offer I will definitely take it! :) I will send you a preliminary PR for you to review because I am not much familiar with most of the chart and probably I will not have time to check all of them once I am done with the TS definitions.

@dupski
Copy link

dupski commented Dec 4, 2018

Same here regarding familiarity @FrankBrullo as I'm only just starting with this library but happy to take a look and give it a test :)

@miglesiasEB
Copy link
Collaborator

Sounds good!

We have a channel in the official d3 slack. Join us there @FrankBrullo !

@miglesiasEB miglesiasEB changed the title Typescript is not supported Support TypeScript Dec 11, 2018
@isaiahtaylorhh
Copy link

Any motion on this?

@Golodhros
Copy link
Collaborator

Not really, a lot of people offered to add the types but nobody actually did!

@ImADrafter
Copy link
Contributor

I think It would be interesting to have this one, since some types can be reused. Anyway, we can start typing the props and the whole config before that gets merged. May I work on this ? :D

@Golodhros
Copy link
Collaborator

Hi @ImADrafter!
I hope we could reuse the types between Britecharts and Britecharts-react, and use them to keep track of how complete are the Britechart React apis.

In my mind, the future should be a monorepo for britecharts, with @britecharts/core, @britecharts/react, @britecharts/types, @britecharts/angular and so on.

If you have time to work on this stuff, I would actually love to get some help on making britecharts/britecharts#868 work.

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

No branches or pull requests

6 participants