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

Add TypeScript typings #15

Closed
SimenB opened this issue Oct 21, 2020 · 7 comments · Fixed by #28
Closed

Add TypeScript typings #15

SimenB opened this issue Oct 21, 2020 · 7 comments · Fixed by #28
Labels
enhancement New feature or request

Comments

@SimenB
Copy link
Member

SimenB commented Oct 21, 2020

It would be very nice if this module came with TS typings 🙂

@guybedford
Copy link
Collaborator

I've marked this as an enhancement issue and will leave it open here. If anyone would like to contribute please do.

@SimenB
Copy link
Member Author

SimenB commented Nov 1, 2020

I was gonna send a PR since the types aren't too advanced, but it seems the functions are different between CJS and ESM with CJS having parse and ESM having {init, parse}.

Thoughts on changing the API to be the same across module types? I.e. changing CJS to

module.exports.parse = parseCJS;
module.exports.init = () => Promise.reject(new Error('init is not supported when using CommonJS'));

Could potentially just no-op in the last one, or warn?

@guybedford
Copy link
Collaborator

@SimenB I can get behind unifying the APIs to do that sure. That sounds like a good suggestion to me.

@SimenB
Copy link
Member Author

SimenB commented Nov 1, 2020

Cool. I can send a couple of PRs tomorrow if you don't get to it.

Current thinking is init should just be Promise.resolve() so you know you can always call it. Without worrying about what API

@SimenB
Copy link
Member Author

SimenB commented Nov 1, 2020

Some time freed up tonight, so sent #27 & #28 👍

@guybedford
Copy link
Collaborator

Thanks for the PRs! I've published a 0.6.0 with these.

@SimenB
Copy link
Member Author

SimenB commented Nov 1, 2020

Thanks @guybedford!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants