Add TypeScript definitions#360
Conversation
|
@superheri This looks like a good start to me! I will take a more in depth look later this week. |
|
@chuckcarpenter since you are likely more knowledgable on TS, perhaps you could take a look? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
@chuckcarpenter @superheri any updates on this? |
|
I was waiting for an answer before continuing. But if you want I can continue slowly during my free times. |
|
@superheri an answer about what? @chuckcarpenter can you please weigh in? |
|
@rwwagner90 About the questions that @chuckcarpenter asked me directly about the code and the reason to do it that way. |
|
@superheri I resolved my initial question based on your answer. If you'd like to keep working on this, we can look at rewriting in TS over time. Sorry for the confusion! |
|
Ok then, I can't promise you that I'll be very fast, but I'll continue to work on this. |
|
@rwwagner90 Seems like it for now. From what I see not everything needs typings since not everything is exposed. Maybe there are some things that should be changed, feel free to tell me if there are anything you want me to change. |
|
@chuckcarpenter do you mind reviewing again please? |
|
@superheri this looks good to me! Have you tried this out with your project to ensure the typings behave as you expect? If so, I think we should be good to merge 😃 |
|
@rwwagner90 I stopped working at my job where I this package is used so I no more have a real usage of it to test it out correctly. So I haven't tried it in a real case. I tried calling the different objects exposed individually in a really small project, but I don't think it is enough. Maybe if one of you has a more complicated demo, I could try that out with it and maybe you could try it out too so we will be sure it works fine before releasing it. |
superheri
changed the title
…making options optional and editing the typings of the Shepherd.d.ts to define it as a class type not the instance of those classes
|
@rwwagner90 Just tried it out with the welcome.js file whan converting it into TypeScript (Testing with TypeScript 3.5.1). (Just realised I had a demo to test it...) I adapted some things and it seems to work with that demo now. However, the demo does not represent every cases and maybe there are some that are not covered. But if you want we could try to deploy it as is for a first version and see if anyone has issues with this. |
|
I think deploying as a first version sounds good 👍 |
3 participants
Trying to implement #359.
For now, only Step types are present as of a base. A couple of things would need to be clarified.
Many TODOs are in the code @rwwagner90 if you have some time, could you give me your thoughts on this start? There are some things I'm not sure about, if you could check the TODOs that would be nice.
If you like how it goes, I would continue with the other files.