Readonly Array for PluggableList

[pcorpet]

Initial checklist

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

Affected package

unified@11.0.5

Steps to reproduce

I'm using the React Markdown library which is using unified as a dependency, especially the plugins. The main component can take a list of plugins with a PluggableList which is mutable. However I believe this is not meant to be modified but to handover an immutable list of plugins.

const REMARK_PLUGINS = [remarkGfm] as const

...
const processor = unified()
    .use(REMARK_PLUGINS)

Before opening a bug at ReactMarkdown, I want to check in here:

• is there an alternative which is readonly for PluggableList? (I could not find one)
• is PluggableList actually mostly used as readonly and you would consider making it ReadonlyArray? at least that the use function would take it as an immutable array?

If you believe that it should actually stay mutable for some reason, I'll reach out to ReactMarkdown.

Actual behavior

This makes the Typescript check fails as REMARK_PLUGINS is readonly (due to the as const) while use expects a PluggableList which is mutable.

Expected behavior

Typescript should not choke and use should accept a readonly array of plugins.

Runtime

No response

Package manager

No response

Operating system

No response

Build and bundle tools

No response

[ChristianMurphy]

@pcorpet what is your concern with using

const REMARK_PLUGINS: PluggableList = [remarkGfm]

or

const REMARK_PRESET: Preset = {
  plugins: [remarkGfm]
}

?

is PluggableList actually mostly used as readonly and you would consider making it ReadonlyArray

Remark itself doesn't modify the plugin array, but we also allow dynamic plugin lists to be generated.

[pcorpet]

I think it's not an issue if this is just one module, however if it's used more widely then it becomes an issue as any user of the array might or might not modify it. I usually think of types in Typescript as a contract. Here when you write that remark doesn't modify the plugin array: it's probably the case today; while having it in the contract (ReadonlyArray) would also ensure it stays the case in the future.

Also I'm adding a linter in our codebase to prevent any React component props to use mutable array because it breaks most rendering behavior if a component modify its own props. I could indeed escape the linter for that occasion, but would prefer to fix the underlying issue: either the array MUST be mutable and then I'll need to create a mutable copy, or the array is only read and can be immutable.

[wooorm]

Readonly types are a bit awkward in TS.
If we’d make PluggableList readonly, then users may run into problems, when they type as PluggableList, and then push to that.
But the intention is further than that: we don’t modify the functions/presets in that array either!
Similarly, while plugins accept readonly options (hopefully, in all cases), if we’d make that type Readonly, that would break users who want to write to that value before passing it to plugins.
So, readonly isn’t only at our boundaries, where we want to be readonly.

Then, there’s the disconnect between TS and “what’s real”. In actual JS you could change that array.

Then there is the overloaded meaning: as const infers things differently, not just readonly but also with more specific types.

Back to your OP, I recommend:

const processor = unified()
    .use(remarkGfm)

You don’t need intermediate variables and types

[pcorpet]

Yes, I see your point: as there is only one type, people use it both for mutable and immutable.

Actually my use case is to use

<ReactMarkdown remarkPlugins={REMARK_PLUGINS}>
  {children}
</ReactMarkdown>

Then the ReactMarkdown library uses unified().use(...) internally.

Would it be possible, without changing the PluggableList type entirely, to create a new type PluggableImmutableList or something like this, and change the interface of the use function to accept it?

[wooorm]

Why does:

const REMARK_PLUGINS = [remarkGfm]

// ...
const processor = unified()
    .use(REMARK_PLUGINS)

Not work for you? Is it your tools? It’s understandable, but I think you’ll find similar things in many libraries, and perhaps configuring your tools is better.

It is indeed possible to make writable and readonly versions of every type. Potentially with help such as from type-fest, I think that has “deep readonly” helpers.
The writable versions as regular exposed types, the readonly versions of accepted parameters.
That is the intention indeed.
But I consider a) as too much duplication, and b) unneeded deps; and both also implying not good enough JSDoc comments / source map stuff?
You’re welcome to try such an approach in a branch on unified, but I think you will reach the same conclusion

[pcorpet]

Yes you're right, I'm finding the same thing in many libraries and I've been updating many of them (examples here and there). I indeed wrote the code the way you have it const REMARK_PLUGINS = [remarkGfm]. I'm trying to get more value from types, especially for interfaces with other libraries. I have found some libraries that actually modified their inputs (d3) and prefer to have a real difference when the library does or does not modify it.

I'll take you on your offer and prepare a branch for a simple change for the use function.

[pcorpet]

@wooorm as you had suggested, I've prepared a branch here: #267

[remcohaszing]

I agree with Christian and Titus. Readonly types are in a weird place. They aren’t really a type and they signal a concept that (mostly) doesn’t really exist within JavaScript. The issue isn’t really related to PluggableList, it’s related to every option accepted by any function that doesn’t mutate its arguments.

Marking an option as readonly is also cumbersome. Users may want to mutate an object before passing it as an argument. In that case they’ll want a writable version of the type.

We could use a deep readonly helper. I’m not sure if it’s worth it. Depending on type-fest just for that adds a big maintenance burden, considering this is about hundreds of packages.

I suggest you write:

const REMARK_PLUGINS: PluggableList = [remarkGfm]

If that’s not an option, you can write:

<ReactMarkdown remarkPlugins={REMARK_PLUGINS as PluggableList}>
  {children}
</ReactMarkdown>