fix: static pages not rendered with fastify after v9.5

[fkatsuhiro]

Changes

issue: #15467

cause:
With the upgrade to Node.js v9.5, a change was introduced in the conditional branching, causing statically built endpoints to be incorrectly routed to SSR processing, resulting in errors due to no executable page function being found.

change:
Changed the relevant conditional statement.

routeData && !(routeData.type === 'page' && routeData.prerender)

↓

routeData && (routeData.type === 'redirect' || !routeData.prerender)

Testing

Docs

[changeset-bot]

🦋 Changeset detected

Latest commit: 7fbe2d5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

[pkg-pr-new]

npm i https://pkg.pr.new/@astrojs/node@15469

commit: 7fbe2d5

[matthewp]

Can you add a test? The condition here doesn't seem to have anything to do with Fastify. What's the underlying problem?

[fkatsuhiro]

@matthewp Thank you for your reply. I had checked comments of #15467.

The original reason for this implementation was a concern regarding the rendering process. Upon comparing the current server-app.ts with version 9.4.6, I noticed that new conditions for pre-rendering had been added. I suspected that !routeData.type === 'page' was the root cause of the error, so I decided to relax the conditions in this implementation.

But now I understood cause of this issue. and I noticed my mistake....

While node v9.5.0 ~ v10.00 -alpha,

astro/packages/integrations/node/src/serve-app.ts

Lines 52 to 63 in 241ffbd

	const routeData = app.match(request, true);
	if (routeData) {
	const response = await als.run(request.url, () =>
	app.render(request, {
	addCookieHeader: true,
	locals,
	routeData,
	prerenderedErrorPageFetch,
	}),
	);
	await NodeApp.writeResponse(response, res);
	} else if (next) {

here
const routeData = app.match(request, true); : this code made initial state of routeData is true. This is the actually cause I think....

But it has been improved already node v 10.0.0 - beta, maybe no problem...

So, my PR no need to reflect for solving this issue.Should I close this PR? Or would you like me to create a test case for this specific issue instead?

[matthewp]

Close the PR if your issue is already fixed

[fkatsuhiro]

Okay. I'll close this PR.