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
Default options for a custom mixed option are overridden then deleted when some, but not all, options are set in typedoc.json #2024
Comments
This is working as expected for mixed options. Mixed options could be objects, numbers, strings, or even a function... for this reason, TypeDoc performs no processing on them. Your plugin will need to do something like: const opts = { ...app.options.getValue("opt"), ...defaults } If your object only contains boolean flags, a Flags option type might be a better fit, TypeDoc does do some additional processing for that case. |
Thank you - your suggested workaround is appreciated. In practice, default options will need to be read in the declaration scope before they are lost, and then merged after the reader is run; app.options.addDeclaration({...});
const defaultOptions = app.options.getValue("opt");
// this below, or in a new scope after app.bootstrap eg. after a renderhook
app.options.addReader(new TypeDocReader());
app.options.read(new Logger());
// end
const myScopedOptions = {...defaultOptions, ...app.options.getValue("opt")} From the theme / plugin developer's perspective, the API method of However, given that;
I assert that the present behavior for mixed options is inconsistent. I expect that there are many reasons why you are implementing the behavior this way, but I argue:
|
I would instead recommend: app.options.addDeclaration({ ..., default: {} }) // no real point including defaults here
// later...
const options = { foo: "a", bar: "b", ...app.options.getValue("opt") } but yes, you have run into a use case which simply doesn't exist for TypeDoc itself, so additional code was not written to cover it. Instead of adding a single |
Appreciated, I also appreciated that, if so required or desired, each option can be individually declared as strings for param parsing. IMO, issues like this one become important for a Typedoc extension's code readability and maintainability. Here is how I am implementing a workaround, which I think is more elegant and succinct for future humans. I am busy kicking the proverbial Typedoc tyres with some plugin development, and then a custom theme development. |
I'm not really opposed to adding a new option type for nested options (how does it merge? don't want to add a new dependency, is |
Agreed. I am starting into a custom theme, which will improve my familiarity with the TDoc codebase. I will come back here for discussion as and when ideas emerge. |
- see [issue 2024](TypeStrong#2024) - only the behavior of plugin options using 'mixed' type is altered It: - stashes the default options of plugins - merges the plugins default and custom options at the end of the bootstrap.
Current FixI have created a Pull Request which works around this issue. It does nothing more than handling 'mixed' type options for plugins by stashing defaults at the beginning of Breaking future updatesMore elegant solutions are possible, but they are going to introduce breaking changes. I have started working on this here.
This will provide a more readable and flexible architecture for plugin/theme developers, and also deprecate some of the obfuscating functions around the read order and double reading of args. This is a bit messy for now, but does function although it wreaks havoc with the whole testing landscape... |
I think both of the proposed options are making this more complicated than it needs to be. A non-breaking way of introducing this behavior that I'd rather go for is:
|
I do appreciate your concerns, and understand that you prefer adding a new behaviour instead of risking interference with existing behavior. This makes sense. I understand your proposal to be as per below:
for (const [key, val] of Object.entries(data)) {
try {
container.setValue(
key as never,
val as never,
resolve(dirname(file))
);
} catch (error) {
ok(error instanceof Error);
logger.error(error.message);
}
}
The default options will now be overridden, and not deleted, in both cases above. Concerning the extensibility of future versions of typeDoc (which may be breaking), the API available to a plugin remains restrictive and inconsistent.There could be any number of reasons why a plugin author may want to, within the bootstrap scope (before the application runs), conditionally and selectively override not only their own, but also the other application options. In this case, they would expect:
I think that much of this can be achieved in a non-breaking way by adding event hooks into the bootstrap process. I suggest considering if your proposal for |
They would only expect this if they had not read the overview of how typedoc works. That is not how it works, and should not be how it works since it means inconsistent behavior for plugins stomping on options. (Which is a bad idea anyways...)
I disagree here... You shouldn't expect
They can do this today, but they have to place a hook in the appropriate place. I would be okay with an event emitted at the end of |
This is not about reading the overview, or any other helpful resources. Typedoc appears to (correctly) make the parsed options immutable in Given that, within bootstrap,
graph LR;
subgraph Bootstrap
Start-->loadPlugins;
loadPlugins-->options.reset;
options.reset-->options.read;
options.read-->End;
end
subgraph run
subgraph app.convert
End-- no existing hooks -->options.freeze;
end
options.freeze-->A[complete run]
end
B{New Hook?}-->End;
loadPlugins-- the user has no control -->options.freeze
IMO, any documentation software package should make the user a first class citizen. The default theme and documenting style / architecture can be as opinionated as desired in the core, but the extensibility should be equally flexible. Yes - allowing a plugin to manipulate options before they are frozen can break things, but equally so for any number of other things a plugin could do, and trying to control that will be like pulling the frayed end of a jumper thread. Yes - there is a meaningful distinction between IMO, this issue can be resolved by solutions already discussed:
Let me know if there is anything you want me to do about this directly. |
Plugins may add readers to the options object, so can technically modify values there... I agree the event at the end of bootstrap is a good idea.
Here's where we disagree. Two examples in TypeDoc's options today: Visibility filters have default values for protected, private, inherited, and external. Users can specify values for these to overwrite the defaults or they could specify Search group boosts today defaults to an empty object, so it would be moot, but I've been considering adding a default to weigh top level reflections higher than properties/methods. You could argue that partial-overrides should apply here, but that either prevents TypeDoc from warning if you specify some custom group which doesn't exist (with I'd be happy to merge a PR adding the hook to bootstrap + a new Object option type, but I don't want to put this functionality on Mixed options. |
Ok, I can now see where changing the behaviour on IMO, the existing behavior you have clarified for "visibilityFilters" and intended for "searchGroupBoosts" is implicit, and I prefer explicit public API's, even if they may need to be more verbose. That's my 2 cent's worth and maybe a debate for another day! 😄 I will get started on a PR for the new event hook and |
Search terms
options addDeclaration custom
Expected Behavior
When adding custom options with
app.options.addDeclaration
, thedefaultValue
fields will be overridden by values intypedoc.json
, otherwise retained as per default.Actual Behavior
If some values are defined in
typedoc.json
, these override the defaults and the remainder are deleted.If an empty object for the custom key is defined in
typedoc.json
, all values are deleted.If no custom key is defined in
typedoc.json
, all values are retained as per defualt values.Steps to reproduce the bug
minimal repro pr
minimal repro
Environment
The text was updated successfully, but these errors were encountered: