A new defineIntegration helper

[florian-lefebvre]

Body

Summary

Adds a new defineIntegration helper to Astro to help integrations authors define integrations in a convenient/robust way.

Background & Motivation

We've been working on the Astro Integration Kit since January with other fellow maintainers and Astro community members. The goal is to improve the DX of integrations authors and make their lives easier, allowing more people to start writing integrations as well.

It's a set of utilities we plan to contribute back to Astro and we're starting today with defineIntegration!

Defining an integration but the DX could be improved using a function helper rather than just having the AstroIntegration type. Also, handling correctly options (so that it's both type and runtime safe) is overlooked by many people. We took inspiration from Starlight and experimented with it for a few months before feeling we have it right, but we need your feedback!

Goals

• Have a function acting as a type helper, similarly to defineConfig
• Allow to type options easily
• Allow (optionally) to have options runtime safety
• Make it backward compatible

Example

Example implementation at withastro/astro#10892 (outdated with this proposal but fairly close)

I originally suggested the zod version only and edited the following examples with alternative options

defineIntegration, the vanilla way

defineIntegration would not be a simple type helper like defineConfig that returns AstroUserConfig, but pretty close. Instead, it would return (options: Options) => AstroIntegration.

const myIntegration = defineIntegration({
	name: "my-integration",
	// options is never in this case
	setup({ name, options }) {
		return {
			hooks: {}
		}
	}
})

myIntegration()

type Options = {
	foo: string
}

const myIntegration = defineIntegration({
	name: "my-integration",
	setup<Options>({ name, options }) {
		return {
			hooks: {}
		}
	}
})

myIntegration({ foo: "bar" })

Note that setup is named after Nuxt Kit defineModule, which is inspired by the vue ecosystem. Happy to rename it.

defineIntegration is already available for usage in AIK, read docs. View source code here.

defineIntegration, the zod way

Matthew had concerns about only providing a zod compatible utility so I introduced the above alternative. However, people wanting to use zod could use a defineIntergation.withZod utility that would behave exactly the same as the AIK version. See docs and source.

Backward compatibility

defineIntegration returns AstroIntegration so it's compatible. We talked about a potential @astrojs/kit package to have backward compatible utilities, but after some discussions with Erika I think it's better not to have it. If integrations authors want to benefit from new hooks params, they should update their peer deps. That should encourage more users to upgrade to more recent versions of Astro, and the process is painless anyway. Note that this means defineIntegration will only be available from the minor it's part of.

[Princesseuh]

+1 😄

I like the @astrojs/kit idea with the re-export. It's a bit wonky for the utilities added on hooks though, I wonder if there's a good way to do this. My own personal suggestion would be to not care too much, as users who are likely to use newer / updated integrations tend to reach updated versions of Astro pretty quickly. Looking at telemetry, 50% of users are not on 4.5 right now.

[ematipico]

I second this idea. The use of @astrojs/kit seems to be a very good comprise!

[FredKSchott]

Love the idea of an official helper! Big +1!

Should this live in Astro though? If I'm designing an integration as an external package, then Astro isn't usually a dependency of my package (I don't think). As an integration author I'd actually prefer it to be an external dependency of my package, and then I could manage major versions myself vs. having to build for the astro@3 interface, the astro@4 interface, the astro@5 interface, etc. etc.

I'd expect this use-case of a defineIntegration helper built-in to Astro to only really be useful for people building integrations in their own app codebase vs. npm packages.

[florian-lefebvre]

In this case, Astro is a peer dependency. So if you want to support only from when it's added, then you add the "astro": "^4.7.0" peer dep and you're good to go. However, it's true to if you want to support older versions then you need @astrojs/kit as a dependency.

To be clear, I feel like defineIntegration should actually be more useful for integrations authors (ie. npm packages).

Idk if that answers your question at all 😅, feel free to re-explain if I didn't get it!

[Princesseuh]

@astrojs/kit would anyway need to depend on Astro for types and stuff, unless we move all of it into a separate package. It always makes sense for integrations to have Astro as a peerDep.

[matthewp]

Thanks for kicking this off @florian-lefebvre! My feedback at this stage is that the goals are a little vague and could use being more specific. A breakdown of what I mean:

• Goal: Improve the DX of defining integrations
  • I think this is mostly to get type inference via the defineIntegration function? Are there other DX improvements that this proposal will provide? If so list them specifically.
• Make it easy to handle options, at both type and runtime levels
  • I don't think I understand this one. Do you mean to handle options that are passed to your integration? If the goal is to make it easier, what is hard about it at present. { arg }: MyOptions seems perfectly easy to me but I'm very likely missing something.
• Promote good practices around the ecosystem
  • What are those good practices that we want to promote?
• Make it backward compatible
  • Do you mean that this proposal intends to add sugar to the existing API without replacing it?

[florian-lefebvre]

I think this is mostly to get type inference via the defineIntegration function? Are there other DX improvements that this proposal will provide? If so list them specifically.

Yes type inference

I don't think I understand this one. Do you mean to handle options that are passed to your integration? If the goal is to make it easier, what is hard about it at present. { arg }: MyOptions seems perfectly easy to me but I'm very likely missing something.
What are those good practices that we want to promote?

Those are actually linked. I didn't want to say that most integrations are not following best practices btw! But what I mean is that setting a type for an integrations options doesn't ensure the value received is actually of that type. TLDR I think it's great to use zod for this. It also allows great pattern with ease (eg. https://github.com/astrolicious/i18n/blob/feat/sitemap/package/src/options.ts#L155-L161)

Do you mean that this proposal intends to add sugar to the existing API without replacing it?

I mean that defineIntegration still returns () => AstroIntegration, so no breaking changes and can be used in older versions of Astro

[matthewp]

Ok, if the goal is to provide validation to integration options then I would say that specifically.

---

I'm not sure how I feel about that goal, to be honest. I think it's possible to overdo using Zod and other validation libraries and this one is close to being that. Not every function in your application should have its arguments validated. There is a performance tradeoff at play here.

[florian-lefebvre]

Yeah I get your point for sure. That being said, idk if performance is really a big issue here since validation only runs when the integration is added to astro (either in a user's config or by another integration), so it should not be called that much, and not at runtime.

[florian-lefebvre]

I updated the RFC, please have another look!

[matthewp]

I still don't think we should do schemas as part of this API. Having the type param as shown in the updated example I think is what we should recommend. I would rather schemas be something that stays inside of a 3rd party library like integration kit. At least for now, we can add layer.

Keeping this API small and focused on providing types for the shape of the integration hooks (as shown in the example) is the way to go, imo.

[florian-lefebvre]

I disagree just like Fry on this point, because of the Astro defaults

[Fryuni]

Yeah, create-astro and all the templates have TS validation disabled by default on the config. The config file is a .mjs and the base tsconfigs have checkJs: false.

The main point of defineIntegration is, IMO, exactly to steer people into validating their options because the type will not be validated by TS unless the user explicitly opts into it.

[Fryuni]

defineConfig and others are useful as no-op type helpers because those are used on default exports or exports where we want to autocomplete something within an inferred type (collections).
In those cases, the user can't write the type anywhere to get IntelliSense, so a type helper is there to save you.
But for integrations, you can just write : AstroIntegration in your function.

If this is the boilerplate we'd show people:

import { defineIntegration } from 'astro';
import { z } from "astro/zod"

const optionsSchema = z.object({
    virtualModuleId: z.string()
});

export default (userOptions: z.input<typeof optionsSchema> = {}) => {
  const options = optionsSchema.parse(userOptions);
  return defineIntegration({
    name: "my-integration",
    hooks: { /* ... */ }
  });
};

I don't think it has any advantage over this:

import type { AstroIntegration } from 'astro';
import { z } from "astro/zod"

const optionsSchema = z.object({
    virtualModuleId: z.string()
});

export default (userOptions: z.input<typeof optionsSchema> = {}): AstroIntegration => {
  const options = optionsSchema.parse(userOptions);
  return {
    name: "my-integration",
    hooks: { /* ... */ }
  };
};

But this is much cleaner, and by requiring validation ensures authors don't miss the fact that types won't help here:

import { defineIntegration } from 'astro';
import { z } from "astro/zod"

const optionsSchema = z.object({
    virtualModuleId: z.string()
});

export default defineIntegration({
  name: "my-integration",
  optionsSchema,
  setup: ({options}) => ({
    hooks: { /* ... */ }
  }),
});

[delucis]

I think “much cleaner” might be overstating it slightly 😁 — in the grand scheme of things once there’s actual code in these snippets, the difference is kind of small. As @HiDeoo mentioned above, plugins like Starlight that already do validation with Zod couldn’t even use the proposed defineIntegration(), because we need access to both raw and validated user config. Of course, it’s possible to add access to that in setup(), but we’re obfuscating stuff that is just regular JS by doing it like that, which is not a great sign.

My instinct is:

• a new API should support functionality that is significantly harder to do without the API
• a new API should be flexible enough to cater for expected needs

I’m not sure if the current design meets those kinds of thresholds.

There are certainly arguments for making nice Zod parse errors easier, but that can be needed all over an integration code base — for example Starlight throws pretty mapped errors in several places — so a generic parseWithfriendlyError() utility could go further than baking it into defineIntegration().

[bluwy]

What if we set // @ts-check for the config by default, or work towards setting checkJs: true by default? Do we still need the zod validation then? If the JS ecosystem go by the rule where every input couldn't be trusted and needs validation, the ecosystem will be very slow.

I get that integrations have a specific target audience, and lives in the Astro config, but you couldn't expect everything the user imports to have runtime validation too. They need to trust TypeScript in some APIs and we could extend that for everything else including integrations.

[bluwy]

TLDR: -1 for me, but I'll explain below 😄

About define*

Stepping back, I think we need to figure out the reason define* API exists today, including defineConfig, defineCollection, defineMiddleware, etc:

• Easy way to define something with types
• Syntax is the same in JS & TS
• Easy to remember, e.g. type define| and get it autocompleted/imported

Otherwise it's much a shell function that doesn't have a lot of utility. It's most useful for end-users, but not much for package authors? So speaking for defineIntegration and defineToolbarApp (which maybe I should've voiced before), the difference between export default define*() and export default {} satisfies X is not a lot. Not to mention, you'd often export a function instead, so it becomes export default () => define*() and export default (): X => {}, which the latter is more natural to me. So some define* API may not make sense.

(I know satisifes X and : X is not exactly the same thing, but I think it's not an issue in practice)

About defineIntegration

I notice defineIntegration is different and it's not a shell function (returns param directly), which conflicts with how the other define* API work. It introduces two syntaxes to author integrations, which I'm not a fan of.

On top of that, I agree with Matthew's comment, and if we remove the schema handling, there's not much left for a sort of defineIntegration API.

[Princesseuh]

I don't know, it seems fairly intuitive to me that an integration would have Astro as a peerDependency, while it's true that this burden exists with runtime import, I also feel like it's fairly minor! (especially if we lead this effort into creating a starter project etc in the future)

[delucis]

On the whole I also agree that we have a convention that our define* helpers are all identity functions that are only used as type helpers (notably in contexts like astro.config.mjs where a user might not be writing TypeScript), so breaking that convention seems something worth discussing, especially if we break it for little added functionality.

[bholmesdev]

@delucis This is a good point on what define* utilities should do. Until now, they were sugar over satisfies (or type casts before satisfies) that work in JS and TS files. Now, defineIntegration() is doing a bit of internal work to potentially parse Zod schemas. defineAction() has a similar issue.

This may be an odd take... but I don't think it matters what the internals of define* actually do. Definitions expand overtime, and I think define has expanded to mean "how you configure a feature in Astro." That feature may be a collection, a database table, an action, or yes, an integration. We may think advocating for manual TS types or create* instead of define* add more clarity. In a way, it actually adds confusion. As a user, you would need to know the internals of a function to discern whether define or create is the import to expect. Users should never have to consider that distinction. I think defineIntegration is the right call to build a configuration helper that we can build upon for future features.

[ematipico]

I somewhat agree, but I'm afraid I also have to disagree. I agree that users should not care about the internals of a function, but Astro is a framework that allows to define third-party plugins, which means that we build an ecosystem.

Eventually, it will be normal for developers to look up to Astro for naming conventions, coding patterns, etc., so the end-users will have a consistent DX.

For example, look at the naming convention set by Rust for function names. I believe they are really good, because via convention, you know what do expect from the naming of function.

Hence, I believe we should be strict here, and come up with a different prefix identity functions that do more than just returning the defined object.

E.g.

• define* -> cheap operation
• create* -> expensive operation, for example internal parsing and transformations

[florian-lefebvre]

Interesting, that means defineAction would have to be renamed

[florian-lefebvre]

I want to get back to this RFC now that I'm mostly done with astro:env. I think an important aspect of defineIntegration is that it much be easy to use for beginners. Nowadays, I may not need it because I'm familiar with the API but it seemed a bit scary at first. I don't know if defineIntegration is the solution to this but I think it can help