Add support for Preact

[vtenfys]

What is the motivation for this pull request?

Support using Preact instead of React

What is the current behavior?

Only React is supported

What is the new behavior?

New option library is added to allow using Preact

Checklist:

• Tests
• Documentation

Any other comments?

As far as I'm aware, NPM doesn't allow specifying multiple packages that can satisfy a single peer dependency (i.e. warn if neither React or Preact are missing, but satisfy if one is present) - so I've used an unofficial option optionalPeerDependencies. Alternatively, package.json could be left as-is, assuming most people will want React.

[coveralls]

Coverage remained the same at 100.0% when pulling 6c6b8ec on davidbailey00:support-preact into b103aae on remarkablemark:master.

[remarkablemark]

Awesome work @davidbailey00

I left a comment/question.

Depending on the approach we take, do you mind adding tests to ensure this library remains at 100% coverage?

Also, could you verify if we need to add any TypeScript definitions here?

[remarkablemark]

I like your proposition @davidbailey00

What do you think of passing the library module as an option? For example:

// html to react
parse('<p>', {
  library: require('react') // default
});

// html to preact
parse('<p>', {
  library: require('preact')
});

// html to custom
parse('<p>', {
  library: {
    cloneElement: () => { /* ... */ },
    createElement: () => { /* ... */ },
    isValidElement: () => { /* ... */ }
  }
});

This way, the API is flexible in terms of SOLID principles and it's easier to support new libraries in the future.

Also, do you prefer naming the new option library or react?

[vtenfys]

Depending on the approach we take, do you mind adding tests to ensure this library remains at 100% coverage?

Also, could you verify if we need to add any TypeScript definitions here?

Absolutely! Will check those things when I next work on this PR :)

What do you think of passing the library module as an option?

I like this approach - initially I thought of passing the module as a string, but I like this approach better since it's easier for bundler optimisation + allows using custom functions which aren't from a single module

Also, do you prefer naming the new option library or react?

Don't mind either way - I was debating this myself when creating the PR. Do you have a preference towards one or the other? If not I'll leave as library

[vtenfys]

Also curious what you think about the optionalPeerDependencies approach, since NPM doesn't have a nice "official" way to handle this

[remarkablemark]

Also curious what you think about the optionalPeerDependencies approach, since NPM doesn't have a nice "official" way to handle this

That's a great question. I'm leaning towards optionalDependencies (see description for npm and yarn). The only thing we lose out is the client warning us if the peerDependency is not installed.

Do you prefer naming the new option library or react?

I'm okay with keeping it named as library.

[vtenfys]

I'm leaning towards optionalDependencies

The issue with optionalDependencies is it forces installing a particular version - the name is a bit misleading since it always installs packages specified unelss there's an error during their installation

[vtenfys]

Just pushed some changes to refactor this - I've made React a peer dependency again, since it's the standard implementation.

[remarkablemark]

Thanks for making the updates!

I've made React a peer dependency again, since it's the standard implementation.

Do you think this would be an inconvenience for packages that don't require react? The only downside would be the warning from npm/yarn install.

Otherwise, I think your changes look good and they will be available in the next release.

[vtenfys]

Do you think this would be an inconvenience for packages that don't require react? The only downside would be the warning from npm/yarn install.

Yeah, that's the only downside - slightly annoying but it's probably best to leave it for the majority of people who will use standard React, and not a major inconvenience

[remarkablemark]

Cool, I'll be merging this PR. Thanks once again for opening this @davidbailey00!

[remarkablemark]

[email protected] has been released and published:

# update with npm
npm i -S [email protected]

# or with yarn
yarn add [email protected]