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

Switch to typescript #178

Closed
wants to merge 5 commits into from
Closed

Switch to typescript #178

wants to merge 5 commits into from

Conversation

sbarfurth
Copy link
Collaborator

@sbarfurth sbarfurth commented Nov 10, 2020

Description

Rewrote all the files in typescript. This adds native support for any typescript applications. I tried to keep the API the same, but that would need to be verified. It would need to be checked if this is a breaking change.

Fixes #28

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have made corresponding changes to the documentation
  • I have added changes to the Unreleased section of the CHANGELOG

@sbarfurth
Copy link
Collaborator Author

Still working on the new build pipeline.

@sbarfurth
Copy link
Collaborator Author

@imbrn Please explain https://github.com/imbrn/v8n/blob/master/src/Rule.js#L29 to me. What is it for? It appears difficult to type and is very non-clear.

@imbrn
Copy link
Owner

imbrn commented Nov 12, 2020

@brfrth actually, I think we shouldn't go with an entire refactor for using Typescript by now. But instead just releasing a version with a v8n.d.ts file. I know it'd be amazing to change the codebase for using that, but it's not that easy as most of the library is dynamic and runtime generated behavior. So starting with only this types file could give us more time and confidence for future refactoring.

What do you think?

@sbarfurth
Copy link
Collaborator Author

Sure, I guess TS would be more of a 2.0 feature. I still want to grasp what exactly that second call to testAux does.

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

Successfully merging this pull request may close these issues.

Support TypeScript
2 participants