"Docs plugin enabled" detection no longer does anything

[Josh-Cena]

Because theme-common has a direct dependency on plugin-content-docs, it is no longer possible to make a site without ever installing content-docs (previously discussed in #3360). Not really a big deal because the user can still install theme-classic and plugin-content-blog and pretend she never pulls in content-docs transitively, but it's still not desirable.

We have some checks in our codebase about whether docs is enabled:

docusaurus/packages/docusaurus-theme-common/src/utils/docsUtils.tsx

Lines 32 to 33 in c8d6c7e

	// TODO not ideal, see also "useDocs"
	export const isDocsPluginEnabled: boolean = !!useAllDocsData;

docusaurus/packages/docusaurus-theme-classic/src/theme/NavbarItem/index.tsx

Lines 18 to 38 in c8d6c7e

	const NavbarItemComponents: {
	// Not really worth typing, as we pass all props down immediately
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	[type in Exclude<Types, undefined>]: () => (props: any) => JSX.Element;
	} = {
	default: () => DefaultNavbarItem,
	localeDropdown: () => LocaleDropdownNavbarItem,
	search: () => SearchNavbarItem,
	dropdown: () => DropdownNavbarItem,
	html: () => HtmlNavbarItem,
	// Need to lazy load these items as we don't know for sure the docs plugin is
	// loaded. See https://github.com/facebook/docusaurus/issues/3360
	/* eslint-disable @typescript-eslint/no-var-requires, global-require */
	docsVersion: () => require('@theme/NavbarItem/DocsVersionNavbarItem').default,
	docsVersionDropdown: () =>
	require('@theme/NavbarItem/DocsVersionDropdownNavbarItem').default,
	doc: () => require('@theme/NavbarItem/DocNavbarItem').default,
	docSidebar: () => require('@theme/NavbarItem/DocSidebarNavbarItem').default,
	/* eslint-enable @typescript-eslint/no-var-requires, global-require */
	} as const;

Because we are now always directly importing these hooks from @docusaurus/plugin-content-docs/client instead of letting the docs plugin register theme aliases, these things always statically exist. We should either remove the checks, or dynamically import content-docs/client everywhere (better IMO in terms of bundle size)

[Josh-Cena]

This is my major dissatisfaction with the ongoing refactors. Everything gets intertwined and our architecture is getting more and more monolithic despite being allegedly "plugin-based".

[slorber]

I don't think it's a big deal considering theme-classic/theme-common are supposed to be an implementation of docs/blog/page content plugins.

There's little value in optimizing install time for users that are not willing to use a specific content plugin IMHO, there are much lower hanging fruits to reduce install time.

#3360 was about a different issue: a blog would not build if the user is using docs: false (despite package being installed => the plugin needed to be registered even if unused)

---

We should either remove the checks, or dynamically import content-docs/client everywhere (better IMO in terms of bundle size)

I agree that we can clean up the lazy NavbarItem require now, also wanted to do this cleanup

Also agree to use dynamic imports, but it requires SSR support for dynamically imported components

---

This is my major dissatisfaction with the ongoing refactors.

I'm not sure to understand what you mean here. What change makes things worse exactly?

Everything gets intertwined and our architecture is getting more and more monolithic despite being allegedly "plugin-based".

I agree with that, but at the same time we have the theme implementing 3 content plugins so it's expected to be coupled to these plugins on purpose.

We could have a more modular architecture, having a plugin being able to register its own navbar item etc (slightly related to the other discussion in #7152 (comment))

But then, if we have 2 themes, then we'll have docs-classic + docs-tailwind + blog-classic + blog-tailwind?

[Josh-Cena]

Yeah, I'm mainly talking about directly importing @docusaurus/plugin-content-docs in the theme, not the fact that the theme provides the docs components.

The theme aliases somehow work like inversion of control. Instead of having direct dependency relationship, the alias allows them to communicate through a shared protocol where plugin provides the data and theme provides the components. There's no hard dependence in this relationship and unplugging anything naturally works.

I'm not asking to split the theme—just that importing from @docusaurus/plugin-content-docs in theme-common has made this coupling much tighter than before. Lazy-loading could fix it.

#3360 was about a different issue: a blog would not build if the user is using docs: false (despite package being installed => the plugin needed to be registered even if unused)

Ah, I understand now. Scratch the point about skipping installation of a package (maybe we still can in the future with optional peer dependencies, but we have been bitten by peer dependencies, so not in the near future). Bundle size is still a valid claim, though

[slorber]

The theme aliases somehow work like inversion of control. Instead of having direct dependency relationship, the alias allows them to communicate through a shared protocol where plugin provides the data and theme provides the components. There's no hard dependence in this relationship and unplugging anything naturally works.

I understand that, but at the same time it can become quite tedious to maintain the "contract" between content plugin and theme this way.

Remember most of these docs apis are internal and undocumented. Adding indirection only increase maintenance cost from us.

These apis are not really meant to be overridden with aliases. If user is unhappy with a behavior, they can swizzle the component and provide a brand new implementation using documented apis like useGlobalData.

I'm not asking to split the theme—just that importing from @docusaurus/plugin-content-docs in theme-common has made this coupling much tighter than before. Lazy-loading could fix it.

I don't understand why it's worse to import it in theme-common compared to theme-classic?

Maybe you are against both, and I can understand that.

IMHO coupling isn't always a bad thing. Here it's done on purpose to avoid the extra cost of an unnecessary indirection layer.

Unless you can give a strong use-case for introducing back this indirection (pragmatic use-case, not theoretical), I'd rather keep it this way for now.

[Josh-Cena]

The use-case is that now theme-common is explicitly dependent on content-docs instead of implicitly through theme aliases, and there's no way to rid content-docs/client from the client bundle now. Previously the @theme alias would expose it as some kind of API because it's swizzlable but that's not the intention—the intention is to decouple so there's no hard dependency. If the user is not using docs, she should not have docs-related hooks in the bundle. Side-effect marker doesn't fix this (I think) because we still eager-load all navbar items.

[slorber]

Then if a user reports using theme-common without using docs (unlikely 🤷‍♂️ ) and complains about bundle size, we can look into it by either introducing back indirection or ensuring tree-shaking works better.

I don't think this is valuable to dedicate much time to this atm, but conceptually I agree.

[Josh-Cena]

Yes, that's basically the idea. Quite annoying (because it signals smelly architecture) but not high-priority. Just here to raise awareness and discuss some solutions.

[slorber]

I guess we can close this now that theme-common does not depend on content plugins anymore

(at least it will be the case in v4 soon: see #10316)