Skip to content

TypeScript: export types of 'domhandler/lib/node' #252

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

Closed
natterstefan opened this issue May 26, 2021 · 8 comments · Fixed by #296
Closed

TypeScript: export types of 'domhandler/lib/node' #252

natterstefan opened this issue May 26, 2021 · 8 comments · Fixed by #296

Comments

@natterstefan
Copy link
Contributor

Hi @remarkablemark,

I think it's best to create an issue for our discussion (starting here) in the merged PR than to discuss further there.

As suggested the solution could be to export the types alongside html-react-parser's types instead of having people installing the package only to use the types.

What do you think?

@remarkablemark
Copy link
Owner

@natterstefan the library is already exporting types in index.d.ts. What type are you suggesting that needs to be exported?

@natterstefan
Copy link
Contributor Author

Hi @remarkablemark,

I think I see my confusion now. When I first had the issue in Next.js and with TypeScript, I saw this comment. The comment connotes that you have to import { Element } from "domhandler/lib/node"; to use the Element type. But in fact (and I was not aware of that until now), you export it already here.

It would probably help to tell people in the README, that for type checking only you do not need to install domhandler, but use the built-in types: https://github.com/remarkablemark/html-react-parser#replace-with-typescript.

It will not help though when people want to check the instance of the element.

Then we are back at the problem that one needs to install another library to do that. 🤔

@remarkablemark
Copy link
Owner

Since v1.2.6, domhandler comes installed as a dependency.

Do we want any additional code or README.md improvements?

@natterstefan
Copy link
Contributor Author

Hi @remarkablemark,

Since v1.2.6, domhandler comes installed as a dependency.

Do we want any additional code or README.md improvements?

To be honest I am not sure. My problem was solved by installing the domhandler package. Only then I was able to check the instance of the element.

It will not help though when people want to check the instance of the element.

This is the actual problem that is left, when we put the types aside (v1.2.6 handles that already). Wouldn't it be possible to export the runtime Element (as described here) as well?

Tree shaking should take care of keeping the bundle size small if not used, doesn't it?

@remarkablemark
Copy link
Owner

That's fair, I'm open to exporting Element. Would you be interested in opening a PR?

@natterstefan
Copy link
Contributor Author

Hi @remarkablemark,

I finally had time to contribute and I've created a PR for you to review here #296.

@remarkablemark
Copy link
Owner

remarkablemark commented Oct 1, 2021

Thanks for the contribution @natterstefan! Your changes will be released in 1.4.0

@natterstefan
Copy link
Contributor Author

Thank you very much and you're welcome, @remarkablemark!

# 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