Merge output: hybrid and output: static #11824

Princesseuh · 2024-08-23T14:31:28Z

Changes

This removes output: 'hybrid' and instead makes static works like hybrid did. If no pages are server-rendered, you still get a normal .html build, like you would with the previous output: 'static', so in theory this is only actually breaking for users of output: 'hybrid'.

To implement this, a new buildOutput property on AstroSettings determine what kind of output should happen, and if it's server you get warned appropriately that an adapter is required for the build to succeed. This property is also used in dev to warn appropriately when using features that require SSR.

The scanning of if a page is pre-rendered or not is now exclusively done during the generation of the route manifest, as opposed to later during the build. This comes with numerous benefits, notably the required ability to know in advance what kind of website we're making before even starting the build, but also the ability to procure more (and accurate) information about the routes earlier anywhere the manifest is used. This could also includes integrations, in the future.

Motivation

The underlying goal of this PR is to promote the usage of SSR sparingly, putting more emphasis on Astro's ability to have only certain pages pre-rendered. We've noticed in our stats and community a great deal of users wrongfully using SSR on every page, slowing down their website unnecessarily.

By having this capacity "unlocked" by default, it also puts greater focus on Astro's ability to build all kinds of websites, not just fully-static ones, which is interesting to explore as Astro grows more and more.

Testing

Tests should pass!

Docs

Oh boy it's a big one for docs! We'll be working on this for the next few weeks and probably more, as this change end up affecting a lot of pages.

changeset-bot · 2024-08-23T14:31:34Z

⚠️ No Changeset found

Latest commit: ef10248

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Fryuni · 2024-08-24T15:21:22Z

packages/astro/src/core/config/schema.ts

-		.union([z.literal('static'), z.literal('server'), z.literal('hybrid')])
+		.union([z.literal('static'), z.literal('server')])


This seems weird. A project with output: 'static' can have a non-static output. When I was reading the thread about this on Discord I thought that the merge would remove static and have hybrid as the default.

The names here honestly don't matter, since they're not used for logic anymore apart from choosing the default value of prerender

My own take is that the key should be changed to something else than output, Matthew felt like it was too confusing for users if hybrid was the default etc. It's still up for debate!

Yeah, I'd prefer to have a boolean prerenderDefault, which defaults to false. No more output.

prerenderDefault forces the concept of prerendering onto people who just want to build a regular static (or server) site. People who build mixed sites are the exception.

I don't feel super strongly about hybrid vs static personally. I think it's ok that static has some non-static output because server can already have non-server output too.

Fryuni · 2024-08-24T16:57:24Z

packages/astro/src/core/routing/manifest/create.ts

+export function resolveInjectedRoute(entrypoint: string, root: URL, cwd?: string) {
+	let resolved;
+	try {
+		resolved = require.resolve(entrypoint, { paths: [cwd || fileURLToPath(root)] });


😭 now I can no longer trick Astro into loading a virtual module as an API route.

But it's fine; this gives me an idea of how to circumvent it again.

This code is not new, it just moved

alexanderniebuhr

We have moved the adapter to https://github.com/withastro/adapters, so this PR can't be merged here anymore.
I would offer to port over the changes to the new repo for you, just let me know if you want me to do that.

Princesseuh · 2024-08-30T10:48:43Z

packages/integrations/web-vitals/src/index.ts

-				if (config.output !== 'hybrid' && config.output !== 'server') {
-					throw new AstroError(
-						'No SSR adapter found.',
-						'`@astrojs/web-vitals` requires your site to be built with `hybrid` or `server` output.\n' +
-							'Please add an SSR adapter: https://docs.astro.build/en/guides/server-side-rendering/',
-					);
-				}
-


This message doesn't make sense anymore, the route that web-vitals adds will cause the website to be hybrid anyway since it's not pre-rendered.

delucis · 2024-08-31T22:31:14Z

there's new buildOutput property on astro:config:done

Hmm, that will be a bit late for Starlight at least – we need to parse user config in astro:config:setup and confirm if it’s OK to inject a route that isn’t prerendered — withastro/starlight#1255 (comment)

And I guess if we inject a route that isn’t prerendered, Astro will switch to “hybrid” so we won’t know it wasn’t hybrid until we made it so.

It’s mainly for error handling in our case, so maybe Astro will give a good enough error anyway? But for stuff like injectRoute() not knowing ahead of time is not ideal.

Princesseuh · 2024-09-02T08:28:36Z

Yeah, Astro will show an error like "You need an adapter to use server-rendered pages" when you try to build.

I noticed during this work that it makes error handling inside of integrations a bit tougher, as many of the errors end up not making sense anymore.

For what it's worth, even in the previous version there was a way for these errors to fail because a later integration, or the adapter (which always ran last) could change the output mode after you checked, which is no longer possible now

packages/astro/src/integrations/hooks.ts

packages/astro/src/core/routing/manifest/create.ts

sarah11918 · 2024-09-02T15:01:38Z

Just noting that docs is (all too) aware of this upcoming change and how it will affect output:* configurations in most Astro projects. This includes streamlining (and updating links) to error messages etc. I believe I am on top of what this means for our regular Astro users who just need to re-understand these build options and how they work.

Where we'll need to be provided explicit documentation is things like e.g. the new buildOutput property on AstroSettings with definition/examples. Anything new introduced, especially as it relates to integration authors, adapters etc. is where you can especially focus your docs efforts.

matthewp · 2024-09-03T12:28:15Z

packages/astro/src/core/build/static-build.ts

@@ -161,7 +155,7 @@ export async function staticBuild(
 			settings.timer.end('Server generate');
 			return;
 		}
-		default:
+		default: // `settings.buildOutput` will always be one of the above, but TS doesn't know that


Isn't it a union of the two strings? TypeScript should know that, and not need the default. See https://www.typescriptlang.org/play/?#code/C4TwDgpgBAzg9gWwsAFgSwHYHMoF4oDkcGEBUAPocAO5wEDcAUIwGYCuGAxsGsVACZwAysDYsWAClSYsALliJk6bAEp5GNggBGEAE5QA3oygnY1NME4opyrCsPHTTzgEMY0IiQLzdyNrowoAEYmJ2c3Dxo6Hz8AqAAmUNMAX0ZkoA

It's not, the default value is undefined, for the short instant where we don't know yet the output.. but we could make it default to the option of the config I think

packages/astro/src/core/routing/manifest/create.ts

alexanderniebuhr · 2024-09-04T18:02:37Z

I was going to resolve my blocking commit, because I saw you opened a matching PR in the adapters repo, but I see that the changes in this PR still include a vercel adapter file?

packages/astro/src/core/routing/manifest/create.ts

removed file

Princesseuh · 2024-09-06T23:08:02Z

There are still some things here to do, but this PR has been open for way too long and conflicts are getting harder and harder to fix. We'll merge it here and fix the few remaining points in later PRs throughout next week.

feat: merge hybrid and static

a33f3ab

github-actions bot added pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Aug 23, 2024

Princesseuh added 3 commits August 23, 2024 16:37

fix: linting

415a980

fix: get a bunch of tests passing

38fe245

Merge branch 'next' into feat/remove-hybrid

032327d

github-actions bot added the pkg: example Related to an example package (scope) label Aug 23, 2024

fix: make forceServerOutput optional

00c0fd2

Princesseuh force-pushed the feat/remove-hybrid branch from c423751 to 00c0fd2 Compare August 23, 2024 23:39

github-actions bot removed the pkg: example Related to an example package (scope) label Aug 23, 2024

Princesseuh added 3 commits August 24, 2024 02:14

fix: more tests passing

dcd3d4d

fix: http2 test

2ae0754

fix: CCC

84b6d23

Fryuni reviewed Aug 24, 2024

View reviewed changes

Merge branch 'next' into feat/remove-hybrid

c9a003d

github-actions bot added the docs pr label Aug 29, 2024

Princesseuh added 8 commits August 29, 2024 11:28

fix: get unit tests passing

afc869b

fix: lint

9fb39b8

fix: vercel

a892e50

fix: build

a7ad179

Merge branch 'next' into feat/remove-hybrid

86923b1

Merge branch 'next' into feat/remove-hybrid

cb8a7c2

fix: build

6478ab4

Merge branch 'next' into feat/remove-hybrid

bcd8280

alexanderniebuhr previously requested changes Aug 29, 2024

View reviewed changes

Princesseuh added 2 commits August 29, 2024 22:08

fix: db tests

4bb31ff

fix: get all normal tests passing

0fce869

Princesseuh commented Aug 30, 2024

View reviewed changes

delucis mentioned this pull request Aug 31, 2024

Add support for SSR withastro/starlight#1255

Merged

3 tasks

bluwy reviewed Sep 2, 2024

View reviewed changes

packages/astro/src/integrations/hooks.ts Outdated Show resolved Hide resolved

packages/astro/src/core/routing/manifest/create.ts Show resolved Hide resolved

packages/astro/src/core/routing/manifest/create.ts Outdated Show resolved Hide resolved

matthewp reviewed Sep 3, 2024

View reviewed changes

packages/astro/src/core/routing/manifest/create.ts Outdated Show resolved Hide resolved

fix: apply feedback

3614071

matthewp reviewed Sep 3, 2024

View reviewed changes

packages/astro/src/core/routing/manifest/create.ts Outdated Show resolved Hide resolved

matthewp reviewed Sep 3, 2024

View reviewed changes

packages/astro/src/core/routing/manifest/create.ts Outdated Show resolved Hide resolved

perf: do in parallel

b6f4db9

Princesseuh mentioned this pull request Sep 3, 2024

feat: remove hybrid withastro/adapters#375

Merged

Princesseuh added 5 commits September 3, 2024 17:07

Merge branch 'next' into feat/remove-hybrid

122b9b7

Merge branch 'next' into feat/remove-hybrid

b5c1d11

fix: tests

27cb9ac

fix: tests, for real

2c4faa7

fix: make server islands tests server-rendered

ca0b570

matthewp approved these changes Sep 4, 2024

View reviewed changes

bluwy reviewed Sep 5, 2024

View reviewed changes

packages/astro/src/core/routing/manifest/create.ts Outdated Show resolved Hide resolved

Princesseuh added 3 commits September 6, 2024 17:03

Merge branch 'next' into feat/remove-hybrid

29b6d56

fix: apply feedback

c878836

nit: remove unnecessary file

2cfe46d

Princesseuh added 2 commits September 6, 2024 17:58

fix: test remove test that abuse prerender logic

e7bd25a

fix: ensure image endpoint is there on dev reload

ef10248

Princesseuh merged commit 50ca656 into next Sep 6, 2024
13 of 14 checks passed

Princesseuh deleted the feat/remove-hybrid branch September 6, 2024 23:07

Princesseuh mentioned this pull request Sep 7, 2024

feat: changesets for the hybrid removal #11941

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge output: hybrid and output: static #11824

Merge output: hybrid and output: static #11824

Princesseuh commented Aug 23, 2024 •

edited

Loading

changeset-bot bot commented Aug 23, 2024 •

edited

Loading

Fryuni Aug 24, 2024

Princesseuh Aug 24, 2024

Fryuni Aug 24, 2024

matthewp Sep 3, 2024

Fryuni Aug 24, 2024

Princesseuh Aug 24, 2024

alexanderniebuhr left a comment •

edited

Loading

Princesseuh Aug 30, 2024

delucis commented Aug 31, 2024

Princesseuh commented Sep 2, 2024

sarah11918 commented Sep 2, 2024

matthewp Sep 3, 2024

Princesseuh Sep 3, 2024 •

edited

Loading

alexanderniebuhr commented Sep 4, 2024

Princesseuh commented Sep 6, 2024

		.union([z.literal('static'), z.literal('server'), z.literal('hybrid')])
		.union([z.literal('static'), z.literal('server')])

Merge output: hybrid and output: static #11824

Merge output: hybrid and output: static #11824

Conversation

Princesseuh commented Aug 23, 2024 • edited Loading

Changes

Motivation

Testing

Docs

changeset-bot bot commented Aug 23, 2024 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderniebuhr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delucis commented Aug 31, 2024

Princesseuh commented Sep 2, 2024

sarah11918 commented Sep 2, 2024

Choose a reason for hiding this comment

Princesseuh Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

alexanderniebuhr commented Sep 4, 2024

Princesseuh commented Sep 6, 2024

Princesseuh commented Aug 23, 2024 •

edited

Loading

changeset-bot bot commented Aug 23, 2024 •

edited

Loading

alexanderniebuhr left a comment •

edited

Loading

Princesseuh Sep 3, 2024 •

edited

Loading