Skip to content

feat(index): export domhandler node types and update README.md #201

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

Merged
merged 7 commits into from
Dec 26, 2020

Conversation

remarkablemark
Copy link
Owner

@remarkablemark remarkablemark commented Dec 26, 2020

What is the motivation for this pull request?

feat(index): export domhandler node types and update README.md

What is the current behavior?

domhandler node types are not exported. Thus, users have to import directly from domhandler (see #199):

import { Element } from 'domhandler/lib/node';

What is the new behavior?

domhandler node types are exported:

import parse, { Element } from 'html-react-parser';

Checklist:

  • Tests
  • Documentation
  • Types

@remarkablemark remarkablemark added the feature New feature or request label Dec 26, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Dec 26, 2020

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
dist/html-react-parser.min.js 7.17 KB (0%) 144 ms (0%) 90 ms (-3.6% 🔽) 233 ms

@coveralls
Copy link

coveralls commented Dec 26, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 057ab48 on feat/types into 3a559e4 on master.

@remarkablemark remarkablemark merged commit b97bc57 into master Dec 26, 2020
@remarkablemark remarkablemark deleted the feat/types branch December 26, 2020 05:02
@remarkablemark
Copy link
Owner Author

Published v1.1.0:

npm:

Yarn:

@bicstone
Copy link

Thanks!

@remarkablemark
Copy link
Owner Author

remarkablemark commented Dec 26, 2020

@bicstone even though I exported the domhandler node types, it still won't help your example in #199 since instanceof requires an object and not a type. So for your use-case, it's still recommended to do this:

import { Element } from 'domhandler/lib/node';
import { HTMLReactParserOptions } from 'html-react-parser';

const options: HTMLReactParserOptions = {
  replace: (domNode) => {
    if (domNode instanceof Element && domNode.attribs) {
      // ...
    }
  },
};

@natterstefan
Copy link
Contributor

natterstefan commented Apr 23, 2021

@bicstone even though I exported the domhandler node types, it still won't help your example in #199 since instanceof requires an object and not a type. So for your use-case, it's still recommended to do this:

import { Element } from 'domhandler/lib/node';
import { HTMLReactParserOptions } from 'html-react-parser';

const options: HTMLReactParserOptions = {
  replace: (domNode) => {
    if (domNode instanceof Element && domNode.attribs) {
      // ...
    }
  },
};

Hi @remarkablemark, wouldn't it be possible to fix that as well? Because now, people need to import from a transcend dependency of this package (rather than only importing directly from this package.

Which can lead to issues like Module not found: Can't resolve 'domhandler/lib/node' (I just got this in my Next.js setup).

@remarkablemark
Copy link
Owner Author

remarkablemark commented May 5, 2021

@natterstefan I can see your point about domhandler being a transitive dependency. My only concern of importing and exporting domhandler is that it will add to the bundle size.

Currently, this approach is only necessary for TypeScript projects. I wonder if there's a better alternative that can achieve the best of both worlds.

@natterstefan
Copy link
Contributor

Hi @remarkablemark,

what if you only import the types of domhandler and also export only the required types? This should not increase the bundle size as it is only available in the generated *.d.ts files. Can you try this?

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

Successfully merging this pull request may close these issues.

4 participants