search

Enteee / plantuml-parser

Parse PlantUML with JavaScript or TypeScript
https://duckpond.ch/category/plantuml-parser
Apache License 2.0
141 stars 33 forks source link

Add types declaration file (resolve #23) #30

Closed geopic closed 4 years ago

geopic commented 4 years ago

Let me know of any errors I have made or any queries you have.

geopic commented 4 years ago

Thanks for the feedback @Enteee , I'll look into it soon.

Enteee commented 4 years ago

@geopic : I would wait right now a bit. I have spent quite a few hours this weekend working on this and I am very close on getting something ready. I was thinking about how we can test if the declared types actually match the data structure returned. Then i found this and I am now slowly converting the project to typescript :) As I said, I am nearly at the point where you could have a look at it.

geopic commented 4 years ago

@geopic : I would wait right now a bit. I have spent quite a few hours this weekend working on this and I am very close on getting something ready. I was thinking about how we can test if the declared types actually match the data structure returned. Then i found this and I am now slowly converting the project to typescript :) As I said, I am nearly at the point where you could have a look at it.

Oh ok, I was gonna get started on shifting each class file in /src to a single .ts file right now haha. Let me know when you're ready for me to take a look.

Enteee commented 4 years ago

@geopic I do think the currently pushed version provides a good base for further changes for a fully typed parser. This would be amazing :rocket: ! Anyways, here a list of things I see we have to do before we can merge this pull request. I am happy to help you, just tell me which points you are interested in doing. I will do the rest:

geopic commented 4 years ago 
* add type declarations for symbols to `conf.js`

Can you go into more detail on what this would entail?

Enteee commented 4 years ago 
* add type declarations for symbols to `conf.js`

Can you go into more detail on what this would entail?

ts-peg.js exposes a configuration option returnTypes. More information here. The idea of them is to make the parser internally typed. For now I would say don't worry about this. I have had a look at how we could use this and it looks like this would need some changes to make this work for us. For example all functions defined on a symbol always return any.... which renders the whole point of those returnTypes declarations a bit pointless. But, I am on this, you can leave this to me. I can provide you more information if you are interested in this as well.

geopic commented 4 years ago

Cool, no worries then @Enteee. Let me know if you'd like me to offer some help in any way.

Enteee commented 4 years ago

@geopic well, there were still the two other topics left to do.

* [ ]  migrate all classes in `src/*.js` files to `src/types.ts` (see examples in `src/types.ts`). _Note_: this also involves changes in `src/plantuml.pegjs` and `src/formatters/graph.js`

* [ ]  make code coverage reports work for typescript files in `src/` (currently it reports coverage for files in `dist/`, which are generated during the build and not checked in)

Also by deleting your repository, you also kind of deleted all the changes that I made to your branch .... :worried:

But thank you for your help and interest in this project. :+1:

geopic commented 4 years ago

Also by deleting your repository, you also kind of deleted all the changes that I made to your branch .... :worried:

Oh, sorry about that :frowning: I hope you still have the changes kept locally...