Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

TypeScript support + ESM module error #2

Open
drwpow opened this issue Oct 29, 2020 · 3 comments
Open

TypeScript support + ESM module error #2

drwpow opened this issue Oct 29, 2020 · 3 comments

Comments

@drwpow
Copy link

drwpow commented Oct 29, 2020

Hello! First of all, wanted to say that I love this package 💯. It’s so useful having a simple API for finding tags in a string of HTML!

I’m trying to use this in a TypeScript Node project, but there aren’t types. I’d love to add them to this package if you’d be open to it.

But then I also noticed that your module export isn’t working: ReferenceError: module is not defined. There’s a module.exports= line in the ESM code, likely generated from microbundle.

So in that light, would you be open to a PR that converts this project to TypeScript? It’d have the following advantages:

  • It’d offer automatic TS types for people
  • It’d fix the ESM export because TS does a better job
  • It would still generate the same CommonJS bundle for Node
  • We’d keep everything the same, from file structure to core logic to output generation (it wouldn’t impact perf at all). We’d only be adding a couple TS annotations in the code that are simply removed at compile time, so the source and output are the same as before (other than fixing the module export as mentioned).

Let me know if you’re open to this! Would be happy to do the work; just wanted to discuss first.

@AndreasPizsa
Copy link
Owner

Hey @drwpow,
thanks for taking the time to open this ticket! My apologies for the long delay, the notification e-mail has slipped through the cracks 😞

I’m totally on board with the change, please feel free to submit your PR.

@drwpow
Copy link
Author

drwpow commented Nov 2, 2020

Awesome. No worries at all 🙂 . Will submit a PR either this week or next

@AndreasPizsa
Copy link
Owner

Hey @drwpow, thanks for the PR!
I’ll review it and get back to you.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants