fix: 13.4.13 handling of middleware

[khuezy]

fixes: #176
Context: Starting with nextjs 13.4.13-canary.0, the middleware handling was removed from next-server, so no middlewares were being executed. Obviously a breaking change if your app depended on middleware handling redirects/rewrites/etc...

TODO:

• figure out a plugin system to only apply this to nextjs 13.4.13+

We can use an esbuild plugin to inject the code conditionally.

@conico974 and I have been brainstorming a solution: https://discord.com/channels/983865673656705025/1137034343580172308

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6b3bd86

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
open-next	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 30, 2023 6:05pm

[khuezy]

We may need to omit some headers.
TODO: double check

[conico974]

There is a x-middleware-override-headers that the middleware set that we may need to use. I have not tested this but the middleware return the overriden headers as x-middleware-request-HEADER_NAME, we might need to apply them ourselves.

I did this, not sure if it's necessary:

const toOverride: string =
  middlewareHeaders['x-middleware-override-headers'];
const overrideHeaders = toOverride.split(',').reduce(
  (acc, cur) => ({
    ...acc,
    [cur]: middlewareHeaders[`x-middleware-request-${cur}`],
  }),
  {}
);

[khuezy]

Is there a good way to trigger that? I'm not seeing the x-middleware-override-headers from the result.response.headers.
In my middleware, I'm setting some test headers via NextResponse.next({headers: {}})

[conico974]

My bad it seems it's only for when we modify the request headers. https://nextjs.org/docs/app/building-your-application/routing/middleware#setting-headers. We still need to do it though

[khuezy]

Thanks for link, I'll add.

[conico974]

I might have an idea for the plugin system. We could write an esbuild plugin like this one https://github.com/zziger/esbuild-ifdef. Instead of just conditionally outputting the code, we could provide an override file which contains the part that we want to override.

We could use it like that

esbuild.build({
    entryPoints: ['./index.js'],
    bundle: true,
    target: 'es6',
    outfile: './out.js',
    plugins: [
        ourPlugin({
            overrideFilePath: "path/to/the/file.ts", 
            // ... plugin config
        })
    ]
});

And on our file and our override file we just override part surrounded like that

/// _Override UNIQUE_ID
const partToOverride = 0;
/// _EndOverride

We could even make it possible to apply this plugin several times for different override

[khuezy]

@conico974, as of the current situation, I think we can move the async function processRequest into its own file, then we can use the "ourPlugin" above to replace the implementation for the specific next version.

eg:

        ourPlugin({
            overrideFilePath: getFilePathForNextVersion(nextVersion), 
            // ... plugin config
        })

If there's major changes in the future that requires a broader scope, we can update it then.

[khuezy]

I tried moving import NextServer from "next/dist/server/next-server.js"; into util.js but esbuild complains b/c it couldn't resolve that path... why doesn't work if that's moved outside the server-adapter?

[conico974]

I tried moving import NextServer from "next/dist/server/next-server.js"; into util.js but esbuild complains b/c it couldn't resolve that path... why doesn't work if that's moved outside the server-adapter?

It might be the other functions that are built (i.e. revalidation function) You probably need to add next in external in esbuild for all the other function since utils is used there.

[khuezy]

Great thanks, that was exactly it. I'll add external: ['sharp', 'next'] to the base esbuild since those should always be external no matter what.

[khuezy]

cc @conico974 , I opted for this implementation instead of the reducer b/c it seemed easier to loop once and apply the x-middleware-request- prefixed headers + headers that were added outside the request object.

[khuezy]

Nextjs uses the DOM lib for the Headers object, but I'm not sure what's the actual correct lib to load since it was still missing partial types for the iterators. But using "DOM.iterator" complains about other things... zzz

[khuezy]

#208