Incremental static builds

[matthewp]

Summary

Incremental static builds, allowing Astro to skip regenerating prerendered pages whose template dependencies and per-path data are unchanged since the previous build.

Links

• Rendered RFC
• Stage 2 Proposal
• Stage 1 Proposal

[spaceemotion]

A couple points coming to my mind:

1. Cyclic Dependencies:

   I can kind of imagine there being quite a few cases where there's a build that makes the module dependency chains kind of cyclic? would that mess up the plans for the cache key generation?

2. Conditional Modules:

   How will the system handle conditional async module loading? I am guessing those are handled by the vite module graph already? (e.g. client scripts that import different scripts depending on backend flags, fetch calls, etc.)

3. Randomness in Assert handling / Code Transformation output:

   I already noticed that the current image pipeline generates different images just by sheer randomness of the optimization process.

   As in; i rebuild and get different images each time (same visual output, just not byte-identical)

[matthewp]

For your first 2 points, the graph doesn't change based on these scenarios. Cyclical dependencies might creates unwanted invalidations; you wind up with more pages being rebuilt than you would like, but from Astro's perspective this is the correct thing to do, as any of those modules changing could affect the output so we must treat it as an invalidation. In reality I think this only happens if you have a module that is used by a lot of pages and it changes often. Like if you somehow added an abstraction on top of content collections itself.

For dynamic imports, it doesn't matter if you conditionally import based on data. The invalidation is based on the graph, so if a dynamic import changes, that page is invalidated. It doesn't wait to see what happens at runtime. This is an extra reminder to make sure the code includes dynamic imports in the graph though.

[matthewp]

About image generation, I wouldn't expect there to be randomness in that process. That sounds like a possible bug, please report if it's something you can recreate.

[matthewp]

In review withastro/astro#17084. it was brought up that middleware can effect output. That's true, middleware can literally do anything, it can mutate every response. So from the framework perspective we can't trust any project with middleware. That would be overkill. I think this is likely a documentation thing, but maybe there's another solution.

[ematipico]

That's odd, though. i18n is purely middleware; does that mean that projects that use i18n routing can't use this feature?

[adamchal]

• [medium][design] packages/astro/src/core/build/incremental-content-collector.ts:13-43 - Content tracking uses one global Set. Concurrent renders overwrite each other's collection, while out-of-process prerenderers such as Cloudflare cannot populate it at all. Because content-data modules are excluded from route hashes, affected pages can remain cached after imported content components change. Tracking must be request-scoped and transported across custom prerenderers.

This finding and f0236de workaround is a bit of a bummer and I’m confused by the finding, because I think it is mixing two things:

1. “Concurrent renders overwrite each other's collection” seems more practical to solve; but
2. “Out-of-process prerenderers such as Cloudflare cannot populate it at all” sounds more like a completely separate issue and is a known-limitation throughout code comments.

Item 2 (out-of-process prerenderers) is annoying. They would need to have a way to inject-back their content collection entries and that inter-process communication is a bit rabbit hole. I agree that these out-of-process adapters are just at-risk at this stage.

However, I am not super familiar with how generate.ts gets multi-threaded when build.concurrency is greater than 1. But, is it too big of an effort to handle multiple writers to the Set()?

Maybe something like beginContentEntryCollection() continues to create the Set() if undefined, but also pushes a deferred Promise.withResolvers() to another globalThis array and returns the resolve/reject methods that generate.ts would use instead of endContentEntryCollection(). I’m not exactly sure where the Promise.all(…) would be awaited safely and performantly, but it might be a safer pattern even in the case of build.concurrency = 1.

[matthewp]

@ematipico No this doesn't affect i18n. What I mean is that middleware could do:

export const onRequest = defineMiddleware((ctx, next) => {
  const response = await next();
  
  if(ctx.url.pathname.startsWith('/foo')) {
    const html = await response.text();
    const newHtml = mutateResponseHtml(html);
    return new Response(html, ....)
  }
})

It can change the response of any page. We only invalidate on cacheKey changes, we don't know about this sort of thing middleware might be doing.

[matthewp]

@adamchal That's a workaround for the experimental release, I want to resolve that before stable. We should stop using global state and concurrency will work just fine, but that's a bigger thing to resolve than I had time for for this release.