Skip to content
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

Allow third-party plugins to override core plugins #2591

Merged
merged 2 commits into from Oct 7, 2021
Merged

Allow third-party plugins to override core plugins #2591

merged 2 commits into from Oct 7, 2021

Conversation

@squidfunk
Copy link
Contributor

@squidfunk squidfunk commented Oct 1, 2021

Summary

This PR introduces a minor change to the get_plugins function, which allows third-party plugins to override plugins in mkdocs.contrib in general and the search plugin in particular.

This was first discussed in squidfunk/mkdocs-material#3053.

Rationale

The search plugin, which is part of the core, is probably the most widely used MkDocs plugin. As we all know, it's enabled by default and needs to be explicitly re-enabled when other plugins are added. This means that many authors use this plugin implicitly and rely on its presence, which is also why it's probably not a good idea to change this behavior afterward.

I think it's safe to say that the MkDocs plugin system really took off. Personally, I think the plugin system is very well designed, as it's super easy to get started, which is why I began writing custom theme-centric plugins as part of Material for MkDocs:

  • tags – allows authors to add tags to the page and integrates with search
  • social – automatically generates social preview images for sharing on social media
  • search – a complete rewrite of MkDocs' own search plugin

Now, the search plugin is the newest addition to the list of theme-provided plugins, and after writing my third plugin, I must say that it makes extreme sense to provide extensions as part of theme-provided plugins. One could argue that plugins should be theme-generic, but some things are just not generalizable. For example, the seemingly unmaintained third-party mkdocs-plugin-tags plugin doesn't allow for injecting tags into pages without using theme extension, and it doesn't further integrate with other features of the site like search. On the contrary, Material for MkDocs own implementation is a fully integrated solution, which doesn't need any customization by the author.

Problem

The new search plugin which I first released as part of Insiders, generates richer previews and cuts indexing time and index size in half (see benchmarks). I've extensively written about it in this blog article, explaining in depth why I believe that the search plugin needed a makeover. It is intended to be a drop-in replacement, as the newest version of Material for MkDocs now doesn't use or support MkDocs own search plugin anymore since it has a quite different indexing strategy. Thus, Material for MkDocs is now incompatible with MkDocs own search plugin, which is also why I've decided that I need to expose the plugin under the same name. From squidfunk/mkdocs-material#3053 (comment):

Why renaming the search plugin is not a good option. So, we could just rename the search plugin or prefix it, right? Theoretically, yes. However, this would probably lead to more confusion, because the search plugin is enabled by default, so users now always have to explicitly list the new search plugin. It would also mean that some templates might break, because we're explicitly checking for the search plugin in some places. Several users override the header partial, so we would have to carry out a major release, with the potential for a lot of issues rolling in, since search is so widely used.

Overloading the search plugin, i.e., exporting an entry point with the same name seems to work in most cases, but not in all. This was first discovered in squidfunk/mkdocs-material#3053 (comment). Sometimes, plugins are loaded in the what I would say correct order, allowing for easy overriding:

[
  EntryPoint(name='search', value='mkdocs.contrib.search:SearchPlugin', group='mkdocs.plugins'),
  EntryPoint(name='search', value='material.plugins.search.plugin:SearchPlugin', group='mkdocs.plugins'),
  EntryPoint(name='social', value='material.plugins.social.plugin:SocialPlugin', group='mkdocs.plugins'),
  EntryPoint(name='tags', value='material.plugins.tags.plugin:TagsPlugin', group='mkdocs.plugins')
]

But sometimes, they are loaded in the opposite order, and I have no idea why:

[
  EntryPoint(name='search', value='material.plugins.search.plugin:SearchPlugin', group='mkdocs.plugins'),
  EntryPoint(name='social', value='material.plugins.social.plugin:SocialPlugin', group='mkdocs.plugins'),
  EntryPoint(name='tags', value='material.plugins.tags.plugin:TagsPlugin', group='mkdocs.plugins'),
  EntryPoint(name='search', value='mkdocs.contrib.search:SearchPlugin', group='mkdocs.plugins')
]

This leads to the problem where MkDocs own search plugin overrides the custom theme-provided one.

Solution

This PR will now prioritize plugins with the same name over those provided in mkdocs.contrib. This allows theme authors to more easily provide custom implementations. After all, search is a functionality that is very theme-centric, because themes need to provide additional JavaScript to make use of the search index.

@squidfunk
Copy link
Contributor Author

@squidfunk squidfunk commented Oct 4, 2021

I intend to merge this in a few days.

@oprypin
Copy link
Member

@oprypin oprypin commented Oct 4, 2021

I would like to check it first, but I haven't done that yet, sorry. Please wait :/

@squidfunk
Copy link
Contributor Author

@squidfunk squidfunk commented Oct 4, 2021

Okay, haven't heard anything and thought you're maybe too busy. A review would be great 👍

@ultrabug
Copy link
Member

@ultrabug ultrabug commented Oct 4, 2021

Same here, rough times for me, will do my best

mkdocs/plugins.py Outdated Show resolved Hide resolved
@oprypin
Copy link
Member

@oprypin oprypin commented Oct 6, 2021

OK, the change makes sense and also I can't see how anything could go wrong with this. I'm sure that overriding plugins wasn't an intended thing (and the particular circumstances for it here irk me), but it is indeed very practical. Thanks for the rationale.

oprypin
oprypin approved these changes Oct 7, 2021
@squidfunk
Copy link
Contributor Author

@squidfunk squidfunk commented Oct 7, 2021

Thanks! I've included your suggestions, so this is ready for merge now. Can we release this as part of 1.2.3? Changes that would be included as I read from the commit history:

The rest looks like chores. Are those save to release?

@squidfunk squidfunk merged commit 4ea78da into mkdocs:master Oct 7, 2021
17 checks passed
@oprypin
Copy link
Member

@oprypin oprypin commented Oct 8, 2021

@squidfunk MkDocs generally seems to try to clump multiple things into a release when possible. But I see a few significant issues to fix, then this Sunday I'll probably make a release.

ultrabug added a commit to ultrabug/mkdocs that referenced this issue Oct 8, 2021
* Allow third-party plugins to override core plugins

* Improve resilience of check for core plugin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants