Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Starlight plugins proposal #753

Closed
HiDeoo opened this issue Sep 24, 2023 · 10 comments
Closed

Starlight plugins proposal #753

HiDeoo opened this issue Sep 24, 2023 · 10 comments

Comments

@HiDeoo
Copy link
Member

HiDeoo commented Sep 24, 2023

A plugin system for Starlight is something that has been discussed for a while on Discord. This issue is a proposal to discuss the potential design of such a system, explain why it could be useful, provide a few examples of what it could look like, and discuss / gather feedback on different APIs that could be exposed to plugins.

Feel free to comment on this issue to share your thoughts and ideas.

Motivation

Starlight is built on top of Astro which already supports Astro integrations to add new functionalities to a project using Astro (Starlight is actually an Astro integration).

Starlight plugins are not meant to replace Astro integrations but rather build on top of them to provide additional functionalities that are specific to Starlight. A common use case that has already been requested multiple times is to be able to read the Starlight configuration from another integration to customize its behavior based on it. This is something that is not possible at the moment and that could be achieved with a plugin system.

Usage

Plugins can be added to a Starlight configuration using a new plugins option. This option would accept an array of plugins just like Astro integrations do.

// astro.config.mjs
import { defineConfig } from 'astro/config';
import starlight from '@astrojs/starlight';
import myStarlightPlugin from 'my-starlight-plugin';

export default defineConfig({
  integrations: [
    await starlight({
      title: 'My Docs',
      plugins: [myStarlightPlugin()],
    }),
  ],
});

APIs

A Starlight plugin requires a name and a plugin function that will be called by Starlight with an object containing various helpers that can be used to interact with Starlight. This function can also optionally return an Astro integration that will be automatically used.

config & updateConfig()

A basic plugin that would require reading & updating the Starlight configuration could be similar to starlight-typedoc which generates some Markdown pages on disk, creates some sidebar navigation items for them and updates the Starlight sidebar configuration to display them.

function starlightTypeDocPlugin(options: StarlightTypeDocOptions) {
  const starlightTypeDoc: StarlightPlugin = {
    name: 'starlight-typedoc',
    async plugin({ config, updateConfig }) {
      // Generate the Markdown pages on disk.
      const pages = await generateTypeDocPages(options);
      // Create a new sidebar group for the generated pages.
      const sidebarGroup = getSidebarGroupFromPages(pages);
      // Add the new sidebar group to the Starlight configuration.
      updateConfig({
        sidebar: [...(config.sidebar ?? []), sidebarGroup],
      });
    },
  };

  return starlightTypeDoc;
}
Show advanced example

A more advanced example could be the one of a plugin like starlight-blog that adds a blog to a Starlight websites. Imagining a world where Starlight component customization has landed, such a plugin would need to update the Starlight configuration to override the <SocialIcons/> component with a custom one that would add a link to the blog and the <Sidebar/> component to use a different sidebar for the blog to list all the blog posts.

It would also need to return an Astro Integration that would inject some new routes to the website to display the blog posts.

function starlightBlogPlugin() {
  const starlightBlog: StarlightPlugin = {
    name: 'starlight-blog',
    plugin({ updateConfig }) {
      // Update the Starlight configuration to override various components and use the one provided
      // by the plugin.
      updateConfig({
        components: {
          SocialIcons: 'starlight-blog/components/SocialIcons.astro',
          Sidebar: 'starlight-blog/components/Sidebar.astro',
        },
      });

      // Return a classic Astro integration that will inject the blog routes.
      return {
        name: 'starlight-blog',
        hooks: {
          'astro:config:setup': ({ injectRoute }) => {
            injectRoute({
              entryPoint: 'starlight-blog/tags',
              pattern: '/blog/tags/[tag]',
            });
            injectRoute({
              entryPoint: 'starlight-blog/blog',
              pattern: '/blog/[...page]',
            });
          },
        },
      };
    },
  };

  return starlightBlog;
}

As expected, a plugin can also return a preset by returning an array of Astro integrations.

State
I have a working branch implementing a prototype of a plugin system with these configuration related APIs. The implementation is not super complex and it's mostly a bit of refactoring of some schemas and workaround some missing features in zod. This would require some polishing and some tests but it feels to me like these APIs should be part of a first iteration of a plugin system considering they have been requested multiple times.

logger

I would love to be able to expose to plugins the AstroIntegrationLogger exposed to integrations in Astro v3:

function starlightExamplePlugin() {
  const starlightExample: StarlightPlugin = {
    name: 'starlight-example',
    plugin({ logger }) {
      logger.info('Computing some things…');
      // …
    },
  };

  return starlightExample;
}
  • In various existing integrations/non-official plugins I created, I almost all the time end up re-installing kleur and creating a custom logger using Intl.DateTimeFormat to get a nice colored output matching the Astro one.
  • We could get out of the box filtering when using the Astro programmatic API with a custom logLevel.
State
Due to some implementation details described later, I don't think this is currently possible.

injectContent()

For plugins that adds dynamic data/pages like starlight-typedoc, having to output files on disk into src/content/docs/ that would either be committed or ignored by the user is not ideal.

Having an API that would allow to add pages to a Starlight website would be a great alternative. This API could be something like:

function starlightExamplePlugin() {
  const starlightExample: StarlightPlugin = {
    name: 'starlight-example',
    async plugin({ config }) {
      // Generate or fetch some markdown content based on the Starlight configuration.
      const markdown = await generateMarkdown(config);

      // Return a classic Astro integration that will add a page to the website.
      return {
        name: 'starlight-example',
        hooks: {
          'astro:config:setup': ({ injectContent }) => {
            injectContent('docs', () => [
              {
                id: 'injected.md',
                slug: 'injected',
                data: {
                  title: 'Injected page with generated markdown content',
                },
                content: markdown,
              },
            ]);
          },
        },
      };
    },
  };

  return starlightExample;
}

Note: technically, this would not require a plugin and should be available to any Astro integration but the example is here to show how it could be used in a plugin.

State
This is something that would require upstream support from Astro and is not something that is currently possible but could be added at a later time.

injectTranslations()

With component customization being worked on and some Discord discussions regarding potentially exposing a module using a more advanced version of the existing i18n t() helper (potentially using i18next), I feel like it would be nice to expose a injectTranslations() function to plugins that would allow them to add translations for custom keys (they would be scoped to the plugin name to avoid conflicts).

For example, internationalization support for starlight-blog is something that has already been requested, and instead of dealing with everything that would be required to support it, I think it could be nice to be able to do something like this:

function starlightBlogPlugin() {
  const starlightBlog: StarlightPlugin = {
    name: 'starlight-blog',
    plugin({ injectTranslations, updateConfig }) {
      // Update the Starlight configuration to override various components and use the one provided
      // by the plugin.
      updateConfig({
        // …
      });

      // Add some translations for this plugin.
      injectTranslations({
        en: {
          'sidebar.recentPosts': 'Recent posts',
        },
        fr: {
          'sidebar.recentPosts': 'Articles récents',
        },
      });

      // Return a classic Astro integration that will inject the blog routes.
      return {
        name: 'starlight-blog',
        // …
      };
    },
  };

  return starlightBlog;
}

and in my plugin custom component I could potentially do something like this:

---
import { useTranslations } from '@astrojs/starlight/i18n';

const t = useTranslations(Astro.props.locale);
---

<div>{t('starlight-blog.sidebar.recentPosts')}</div>
State
If this is something that is wanted, this would requires some important changes in the Starlight i18n mechanism to support this (out of the scope of this issue) but could definitely be added at a later time.

injectSettings()

For plugins accepting some configuration options that needs to be read in some Astro components, the go-to solution at the moment is to mimic what Starlight is currently doing for its configuration and expose the plugin configuration through a virtual module that can be imported in the components.

This works great and this what starlight-blog is currently doing for example but this adds some complexity, requires some boilerplate and feels like something that would be repeated a lot for all plugins doing something similar. Having a injectSettings() function that would allow to define some settings that would be injected automatically by Starlight in the existing settings virtual module could be a nice alternative (they would be scoped to the plugin name to avoid conflicts).

function starlightExamplePlugin(options: StarlightExampleOptions) {
  const starlightExample: StarlightPlugin = {
    name: 'starlight-example',
    plugin({ injectSettings }) {
      // Validate the plugin configuration and do some other things.
      // …

      // Expose the plugin configuration to the components.
      injectSettings(options);
    },
  };

  return starlightExample;
}
State
If this is something that is wanted, I am not sure this would be something that would need to be shipped in the first iteration but could definitely be added at a later time to improve the plugin system DX.

astro add

Plugins shipping an Astro integration should not use the astro-integration keyword to support automatic installation using astro add as this would install the plugin in the Astro configuration integrations array instead of the expected Starlight configuration plugins array.

At a later stage, if this is something that is wanted, we could potentially add a new starlight-plugin keyword to support this with astro add but this feels like something that is not needed at the moment and could be added later.

Implementation details

This section is mostly here to explain some implementation details so feel free to skip it if you are not interested.

Show implementation details

Object vs function

I decided to choose an object over only a function to define a plugin for my initial prototype as it felt like a more natural choice considering that Astro integrations are also defined as objects.

This also provides an easy way to add a name to the plugin which could be used to automatically scope translations and settings injected with injectTranslations() and injectSettings() if these APIs are added and potentially support more metadata in the future if ever needed.

Architecture

The current architecture of the Starlight integration could be heavily simplified to something like:

export default function StarlightIntegration(opts: StarlightUserConfig): AstroIntegration[] {
  // Parse and validate the user configuration.
  const userConfig = StarlightConfigSchema.safeParse(opts);

  const Starlight: AstroIntegration = {
    name: '@astrojs/starlight',
    hooks: {
      'astro:config:setup': ({ config, injectRoute, updateConfig }) => {
        // Inject the Starlight routes.
        // …
        // Update the Astro configuration (remark/rehype plugins and exposes the Starlight
        // configuration through a virtual module).
        updateConfig({
          vite: {
            plugins: [vitePluginStarlightUserConfig(userConfig, config)],
          },
          // …
        });
      },
      // …
    },
  };

  // Returns a preset containing the Starlight integration and some other ones that uses the
  // Starlight configuration.
  return [starlightSitemap(userConfig), Starlight, mdx()];
}

Considering that the Starlight user configation needs to be passed down to some other Astro integrations returned by the Starlight integration factory function and that plugins can also returns Astro integrations, running the plugins cannot be done the Starlight integration astro:config:setup hook as it would be too late. This means that the plugins need to be run before the Starlight integration is returned.

This also explains why the AstroIntegrationLogger cannot be exposed to plugins as it is only available in an integration hook.

A simplified version of an implementation could be something like:

export default async function StarlightIntegration({
  plugins,
  ...opts
}: StarlightUserConfig & StarlightPluginsUserConfig): Promise<AstroIntegration[]> {
  // Validate the plugins configurations
  const pluginsConfig = StarlightPluginsConfigSchema.safeParse({ plugins });

  let userProvidedConfig = opts;
  const pluginsIntegrations: AstroIntegration[] = [];

  // Loop through the plugins and run them.
  for (const { plugin } of parsedPluginsConfig.data.plugins) {
    // Run the plugin and accumulate the integrations it returns.
    const integrations = await plugin({
      config: userProvidedConfig,
      updateConfig: (configUpdates) => {
        // Update the user provided configuration with the updates from the plugin.
        // …
      },
    });

    if (integrations) {
      // Add the integrations returned by the plugin to the list of plugin integrations.
      // …
    }
  }

  // Parse and validate the user configuration updated by the plugins.
  const userConfig = StarlightConfigSchema.safeParse(userProvidedConfig);

  const Starlight: AstroIntegration = {
    // Same as before.
    // …
  };

  // Returns a preset containing the Starlight integration, some other ones that uses the Starlight
  // configuration and the plugins integrations.
  return [starlightSitemap(userConfig), Starlight, mdx(), ...pluginsIntegrations];
}

Considering that a plugin can be asynchronous, this means that the Starlight factory function needs to be asynchronous as well. This means that the basic Starlight setup would need to be updated to support this by awaiting the Starlight factory function.

export default defineConfig({
  integrations: [
    // The `await` keyword would be required to support plugins.
    await starlight({
      title: 'My Docs',
      plugins: [myStarlightPlugin()],
    }),
  ],
});

Note that this could potentially be avoided by an upstream change in Astro to support resolving promises provided to the integrations array but this is not something that is currently supported.

Another last thing to note with this implementation is that the read-only copy the user-supplied Starlight config shared with a plugin would be relying on the order of the plugins array. I guess that if we wanted to expose a copy of the configuration after all plugins have run, we would need to use a hook mechanism similar to the one used in Astro integrations.

@delucis
Copy link
Member

delucis commented Sep 25, 2023

Incredibly detailed write-up, thank you @HiDeoo! Overall this looks very much aligned with what I had imagined and I agree on all the various possibilities for enhancement and what can be left until later.

Some quick notes:

  • I might suggest an addIntegration() helper method over returning integration(s) directly. What do you think? Returns are always a bit tricky because by definition you can only use them once. A hook would let us collect the integrations that need adding and then deal with them.

  • Am I right that being forced to return integrations (from Starlight) is a key driver to a few architectural issues here? I wonder if it would be possible to fix Astro such that updateConfig() calls that add integrations see those integrations handled, so Starlight isn’t forced into that pattern. This would have some benefits to Starlight as well as the plugin architecture, because we could do stuff like avoid injecting sitemap or mdx if a user already has those configured. If I’m correct on that, I’ll chat a bit with the Astro team and see what’s possible. (I think that would also solve the async question because we could then run plugins in astro:config:setup which is allowed to be async already.)

@HiDeoo
Copy link
Member Author

HiDeoo commented Sep 26, 2023

Thanks, glad to hear it looks like we are on the same page.

I might suggest an addIntegration() helper method over returning integration(s) directly. What do you think? Returns are always a bit tricky because by definition you can only use them once. A hook would let us collect the integrations that need adding and then deal with them.

I think that's a really good idea. It's even more useful if the user want to conditionally add integrations as a plain if statement is always easier than doing that in a returned array I guess. This would also simplify a bit the typing of a plugin.

Am I right that being forced to return integrations (from Starlight) is a key driver to a few architectural issues here?

Definitely.

because we could do stuff like avoid injecting sitemap or mdx if a user already has those configured

I think this could work for basic cases but if I am not missing something, this would not work for some more advanced cases, e.g. sitemap being itself added by another integration due to the fact that at that point in astro:config:setup, the config passed down to this hook is resolved before any other integrations have run so there would be no guarantee that the user does not have sitemap or mdx in their final config?

@delucis
Copy link
Member

delucis commented Sep 26, 2023

I think this could work for basic cases but if I am not missing something, this would not work for some more advanced cases

Yes, that’s correct. Would only work for config like [sitemap(), starlight()] not the reverse of that. But we could use our current approach of throwing in astro:config:done to inform people.

throw new Error(
`Found more than one instance of ${builtin}.\n` +
`Starlight includes ${builtin} by default.\n` +
'Please remove it from your integrations array in astro.config.mjs'
);

We could update this error to tell people to remove OR move before starlight in the array.

Edit: Actually, I think my example there would be handled. The only issue would be multiple integrations adding clashing integrations dynamically. You’d need to rely on any one integration to be authored responsibly and check before auto-injecting an integration.

@HiDeoo
Copy link
Member Author

HiDeoo commented Sep 26, 2023

Edit: Actually, I think my example there would be handled. The only issue would be multiple integrations adding clashing integrations dynamically. You’d need to rely on any one integration to be authored responsibly and check before auto-injecting an integration.

Yeah, that was the example I was thinking about but I guess this would be pretty rare and if this is a pattern supported by Astro, if you are an integration injecting multiple integrations, it would maybe be best practice to check if the integration is already injected.

So I guess this would all make sense if this usage is supported by Astro and we could indeed move the async part in astro:config:setup.

@delucis
Copy link
Member

delucis commented Sep 26, 2023

Opened a PR/proposal that would unlock this: withastro/astro#8672

@yanthomasdev
Copy link
Member

I really like what's written here and it would perfectly work for my future use-case of a Starlight plugin system (no spoilers!)

I know it's not adding much but just to be vocal that I am +1 on the suggestions and enhancements 🙌

@delucis
Copy link
Member

delucis commented Sep 27, 2023

Noting that withastro/astro#8672 was merged and will be released in Astro 3.2!

This will unlock:

  • Refactoring Starlight to avoid the need to return an array of integrations (will let us solve Starlight with an existing sitemap integration #717 for example)
  • Designing the plugin API here much more easily as we’ll be able to run plugins from astro:config:setup instead of earlier

@spaceemotion
Copy link

Will this also make it possible to have a custom 404 page? right now I am "patching" the plugin by hand so the call gets redirected to my own site:

const patchStarlight = (instance) => {
  const [sitemap, plugin, mdx] = instance;

  const patched = {
    ...plugin,
    hooks: {
      ...plugin.hooks,
      'astro:config:setup': ({ config, injectRoute, updateConfig }) => {
        const injectPatched = (route) => {
          if (route.pattern === '404') {
            // do not overwrite 404 page
            return;
          }

          return injectRoute(route);
        }

        plugin.hooks['astro:config:setup']({ config, injectRoute: injectPatched, updateConfig });
      },
    },
  };

  return [sitemap, patched, mdx];
};

is this something the new API can help out with?

@delucis
Copy link
Member

delucis commented Sep 27, 2023

Ah, no that's a separate issue @spaceemotion — we need to investigate a) whether we can move to a single injected route instead of injecting 404 separately (last I checked we needed to do this because a 404 route generated from a dynamic route was ignored by Astro's Dev server) and b) if not, probably add an official config option to disable injecting the 404.

@HiDeoo
Copy link
Member Author

HiDeoo commented Sep 28, 2023

Noting that withastro/astro#8672 was merged and will be released in Astro 3.2!

I've been tracking the issue silently, super excited to see this change shipped so quickly, such a great job! 🎉

Ah, no that's a separate issue @spaceemotion — we need to investigate a) whether we can move to a single injected route instead of injecting 404 separately (last I checked we needed to do this because a 404 route generated from a dynamic route was ignored by Astro's Dev server) and b) if not, probably add an official config option to disable injecting the 404.

Posted a bit of my initial investigation regarding the first point on Discord but did not have the time to follow up on it. Cross-posting here for reference:

The issue in dev is that this would not get picked by the getCustom404Route()

As we would want to hit getStaticPaths() for this to work, I guess we would need to re-loop over preloaded route matches like it's done a bit earlier but this time with a pathname being '/404' before calling getCustom404Route() if no match is found.

The extra loop may be fine (?) considering it uses the RouteCache? I think the 404.astro would also need to use Astro.response.status = 404; otherwise it would be a 200 status code. I may also be missing something here as it was a quick investigation ^^

@withastro withastro locked and limited conversation to collaborators Oct 23, 2023
@delucis delucis converted this issue into discussion #962 Oct 23, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants