Fix prop types of plugins

[starpit]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

This PR extends the prop-types of rehypePlugins and remarkPlugins to support strings and arrays of strings. See remarkjs/remark#945

[codecov-commenter]

Codecov Report

Merging #683 (8a6886b) into main (8143c12) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #683   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          731       765   +34     
=========================================
+ Hits           731       765   +34     

Impacted Files	Coverage Δ
lib/react-markdown.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8143c12...8a6886b. Read the comment docs.

[wooorm]

Can you add a test, that failed before, and is now solved?

[starpit]

Sure thing! I have updated the PR with tests.

[ChristianMurphy]

are there any example of plugins that use bool or number that we could test with?

[starpit]

Not on our side, but it seemed worth having the completeness? There is nothing wrong with a plugin taking numbers or booleans as options?

[wooorm]

@ChristianMurphy You previously said:

• […] I don't see any as being a good fit

• I'm […] cautious adding […] complexity to the prop types, […] the TypeScript types offer a more precise understanding of what is accepted.

I feel most for the latter. In particular because TS types are better and this current prop-types code is overlong, what about using “any”s here?

[starpit]

I agree that any is a better fit. Isn't it what the typescript type declarations have, anyway? Is this the right type to be looking at?

type Pluggable<PluginParameters extends any[] = any[]>

[ChristianMurphy]

I agree that any is a better fit. Isn't it what the typescript type declarations have, anyway? Is this the right type to be looking at?

Not quite. Walking through that:

<PluginParameters

Means that TypeScript wants a specific type

extends any[]

means that TypeScript means that a plugin can have from 0 to any number of settings

= any[]>

means for plugins without typings, don't type check.

Practically that means TypeScript will match an exact type based off what the plugin passes, but can fallback for legacy plugins which are untyped.

---

[…] I don't see any as being a good fit

I'm […] cautious adding […] complexity to the prop types, […] the TypeScript types offer a more precise understanding of what is accepted.

I feel most for the latter. In particular because TS types are better and this current prop-types code is overlong, what about using “any”s here?

I'd see more a balance of these.
Maybe using any for one of the inner-more types?
For example:

  // Plugin options:
  remarkPlugins: PropTypes.arrayOf(
    PropTypes.oneOfType([
      PropTypes.object,
      PropTypes.func,
      PropTypes.arrayOf(
        PropTypes.oneOfType([
          PropTypes.string,
          PropTypes.object,
          PropTypes.func,
          PropTypes.arrayOf(
            PropTypes.any
          )
        ])
      )
    ])

[wooorm]

That is also acceptable to me. Boolean is most important, but a) I’ve also seen userland plugins accept other random stuff (which is slightly weird so it’s not very important), b) the more stuff is added, the closer it becomes to any anyway

[starpit]

I pushed this update. I had to add a pragma around the use of PropTypes.any:

// type-coverage:ignore-next-line  

[starpit]

well that's bizarre. the code formatter you use changed the case of the pragmas, thus breaking the pragmas. Adding

            // prettier-ignore                                                                                        
            // type-coverage:ignore-next-line   

[wooorm]

The ESLint rule turned on by xo does unfortunately not understand type-coverage comments.
https://github.com/xojs/eslint-config-xo/blob/0a302bdd73bfd49e956ec7f5ebf2c2247c62c310/index.js#L274-L284.
You could try and PR it to them, or you can turn it off here.

[starpit]

I have updated the PR to disable the code formatting rule for the type-coverage line.

[wooorm]

Thanks, released!