An error occurs when using "typedoc-plugin-external-module-name" to prompt "Error: The component `s'has been added."

[Houfeng]

An error occurs when using "typedoc-plugin-external-module-name" to prompt "Error: The component `s'has been added."

[aciccarello]

Could you share how you are getting this error? I'm curious why it thinks it has been added twice.

[justrhysism]

@aciccarello I experience this when using gulp watch to rebuild the docs (using gulp-typedoc).

The error isn't thrown on the initial build, but on subsequent builds. gulp-typedoc creates a new instance, so one would assume that everything internally is "fresh".

typedoc-plugin-external-module-name simply just adds itself via addComponent.

It appears as though the typedoc application lifecycle isn't ending. I'm not an expert on the application side of things, but I feel like it shouldn't end and restart all the time, so if this decision was deliberate and it's by design, that makes sense - however if this is the case, I don't believe that it should be up to plugin authors to check to see if their plugin/component has already been added.

@Houfeng's PR Houfeng@625c429 addresses the issue by simply not throwing an error if a plugin has already been added. I'm not sure what the architectural decision was to throw an Error here was, but it does seem a little odd. I've suggested that maybe simply verbose logging the situation and silently continuing may be a better alternative.

[aciccarello]

@justrhysism This sounds like it could be a gup-typedoc specific issue. Typedoc currently isn't set up for multiple builds.

[justrhysism]

gup-typedoc specific issue

Is it, though? I don't know for sure, but my gut feel is that it's simply exposed by usage with gulp. Maybe someone with a better idea of the inner workings of gulp pipes may be in a better position to suggest otherwise, but the instance instantiation:

const app = new typedocModule.Application(options);

is encapsulated in a function, so once that function ends, app is discarded, and with it, I would assume, the Application.

Typedoc currently isn't set up for multiple builds.

Maybe not concurrent builds, but in my particular use-case which exposed this issue, builds were only being triggered in a serial fashion (i.e. no more than one build occurring at any one time).

---

Specifics aside, what is the design decision behind throwing an error when attempting to add the same plugin twice? What, if any, would be the negative impact of simply ignoring any later attempts to add an existing plugin?

[justrhysism]

Okay, trying to dig into the underlying issue - I don't really understand the codebase, but setting loads of breakpoints and exploring, I have learnt a few things:

1. When TypeDoc is first run (before Application instantiation), each component is added to host._defaultComponents
2. When the Application is instantiated, all default components are initialised
3. Plugins are discovered and added
   1. I'm not sure what the standard method for plugins adding themselves is, but typedoc-plugin-external-module-name adds itself as a component via app.converter.addComponent
   2. This in turn adds itself to the _defaultComponents (which has a longer lifecycle than the Application)
4. If another Application is instantiated, steps 1-3 are run again - however since typedoc-plugin-external-module-name is in the _defaultComponents list, it's already added to _componentChildren by the time the plugins are discovered and added.
   1. When typedoc-plugin-external-module-name attempts to add itself again, an error is thrown because it's already in the childComponents list.

Digging around further, I can see why it's been architected this way - single "pipeline" of converters regardless of how they arrived into the pipeline - I think attempting to change how any of the component systems work to "resolve" this is just a very deep and unnecessary rabbit-hole.

Through this discovery, I can't see any reason why an error would need to be thrown if a component of the same name added twice (and prevented) - except maybe during development.

[aciccarello]

Closed via #847 using the simple fix since a comprehensive solution will be messy. Left a TODO note to indicate that the return is not ideal.