Skip to content

feat: build option extractMiddleware#7448

Merged
ematipico merged 4 commits intofeat/vercel-edge-middlewarefrom
feat/vercel/split-middleware
Jun 26, 2023
Merged

feat: build option extractMiddleware#7448
ematipico merged 4 commits intofeat/vercel-edge-middlewarefrom
feat/vercel/split-middleware

Conversation

@ematipico
Copy link
Copy Markdown
Member

@ematipico ematipico commented Jun 22, 2023

Changes

This PR adds a new build option called build.extractMiddleware. This is an advanced option needed for integrations, and it will be used later on in the Vercel adapter.

Testing

I created a small test case, asserting that the locals won't be rendered because the middleware it's not called.

Docs

If the team agrees, I'd like to skip the doc review for now, and review it only when the final code is ready to be merged on main

@ematipico ematipico requested a review from a team as a code owner June 22, 2023 15:15
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jun 22, 2023

🦋 Changeset detected

Latest commit: ac4d948

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

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release labels Jun 22, 2023
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@ematipico ematipico force-pushed the feat/vercel/split-middleware branch from 26bbfda to 34154dc Compare June 22, 2023 15:17
@matthewp
Copy link
Copy Markdown
Contributor

Happy to see this was maybe a smaller change. But it looks like from the code that if opts.settings.config.build.splitMiddleware is true, then middleware will not be built because it's not part of the module graph. Doesn't it need to become a top-level input? Or no?

@ematipico
Copy link
Copy Markdown
Member Author

ematipico commented Jun 23, 2023

Doesn't it need to become a top-level input? Or no?

It seems there's no need, the code emitted will look like this:

import { e as sequence, f as defineMiddleware } from './chunks/astro.52e57b1a.mjs';

const third = defineMiddleware(async (context, next) => {
	if (context.request.url.includes('/broken-locals')) {
		context.locals = {
			fn() {},
		};
	} else if (context.request.url.includes('/does-nothing')) {
		return undefined;
	}
	return next();
});

const second = defineMiddleware(async (context, next) => {
	if (context.request.url.includes('/second')) {
		context.locals.name = 'second';
	} else if (context.request.url.includes('/redirect')) {
		return context.redirect('/', 302);
	}
	return await next();
});

const onRequest = sequence(first, second);

export { onRequest };

And it won't be imported by any page

@ematipico ematipico force-pushed the feat/vercel-edge-middleware branch from 99f5d26 to c097653 Compare June 23, 2023 08:20
@ematipico ematipico force-pushed the feat/vercel/split-middleware branch from 34154dc to 594401f Compare June 23, 2023 08:20
@ematipico ematipico force-pushed the feat/vercel/split-middleware branch from fe4336f to 7671c6e Compare June 26, 2023 10:21
@ematipico ematipico requested a review from bluwy June 26, 2023 10:21
@ematipico ematipico force-pushed the feat/vercel/split-middleware branch from 7671c6e to 4f8da57 Compare June 26, 2023 10:48
@ematipico ematipico changed the title feat: build option splitMiddleware feat: build option extractMiddleware Jun 26, 2023
@ematipico ematipico force-pushed the feat/vercel/split-middleware branch from 4f8da57 to ac4d948 Compare June 26, 2023 12:02
@ematipico ematipico merged commit a31db90 into feat/vercel-edge-middleware Jun 26, 2023
@ematipico ematipico deleted the feat/vercel/split-middleware branch June 26, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants