Allow userOptions to nullify certain keys in a plugin's defaults

[folliehiyuki]

Enter your suggestions in details:

A Lume plugin can allow users to supply additional options for the underlying program it utilizes. The way official plugins handle deep merging is by using the internal merge() function, which discards keys with undefined values. This is fine for most cases, except when the underlying program has a mandatory but mutually-exclusive set of options (e.g. esbuild). If the default settings contain 1 of these, the other keys cannot be specified in the user options.

This is not a Lume issue. Generally the plugin's author should take care of it. Despite that, I'd love to have a standard way for them to handle the situation gracefully, since most of the time the plugin can't aggressively modify the underlying program's option set.

[oscarotero]

So, basically you want to remove these lines of code, right?

Well, it makes sense, I guess when users set an option as undefined, they expect to have this option as undefined, for example:

site.use(esbuild({
  options: {
    minify: undefined
  }
}));

But I can't imagine a scenario where you can do this. Plugins tipically have sensible defaults, adapted to their goal. For example, the default options of esbuild plugin are focused on output js for browsers, and the use of this plugin to output code for Node (for instance) doesn't make sense in this context (a static site generator). You can override the option with other value, but not remove the option.

Do you have a specific use case where this is needed?

[folliehiyuki]

Do you have a specific use case where this is needed?

I raised this issue while working on a plugin for https://shiki.style/. Its highlight function requires either a theme string or themes: { light: string, dark: string } object to be set in the arguments (https://shiki.style/guide/dual-themes), so I have to put in something as the default. Luckily, in this case, the function disregards theme key if themes object is present, so default to { theme: "nord" } for example does the trick. With that said, if my plugin sets themes in the default settings instead, the user won't be able to use a single theme for their website.

For esbuild, outdir and outfile options conflict with each other. A user with only a single JavaScript source file in their website might decide to set outfile and the build will fail. This is unlikely to happen, though, since it's better to use site.add() to rename the source file.

[oscarotero]

A solution is, instead of exposing the shiki options in the plugin options, define custom options and pass them to shiki. For example:

export interface Options {
  theme: string | { light: string, dark: string }
  // other options...
}

Another option is to don't have any value in the defaults object and assign the default value later:

if (!options.theme || !options.themes) {
  options.theme = "nord"
}

[folliehiyuki]

Yes, this is an easy problem to solve on the plugin side, but it requires additional processing of the user's provided options. I'm advocating for a "lazier" method of able to just throw what the user has set to the underlying program after the initial deep merge, and expose it so community plugins can reuse. The user can cherry-pick options intentionally to fit their own use case.

As I described, this is not an issue (why I chose enhancement tag in the 1st place), so feel free to close if you think it's not worth the code change.

[oscarotero]

I'm not sure if this is a good idea, because it requires to the user to know the default options in advance in order to manipulate it. For example in you case:

site.use(shiki({
  options: {
    themes: { light: "foo", dark: "bar" }, // asign the new theme
    theme: undefined, // remove the default theme assigned by the plugin
  }
}));

To me, this is confusing for the consumer of the plugin. I think options should be additive and only add or change values. Setting a undefined value to disable something seems a bit hacky to me.

Anyway, I'll think more about this and I'm going to leave this issue open, waiting to hear more opinions.
Thanks!