Headers from next.config override headers defined in a middleware

[dirkdev98]

Setting default response headers in the next.config are not overwritten by response headers set in a middleware. Based on the docs and previous deployments in a standalone builds I'd expect this to work.

I think the offending lines in OpenNext are here:

opennextjs-aws/packages/open-next/src/core/routingHandler.ts

Lines 132 to 135 in fe913bb

	headers = {
	...middlewareEventOrResult.responseHeaders,
	...headers,
	};

I could send a PR, however I think this is a breaking change for existing users, so might not be ideal to land in v3?

[sommeeeer]

You have pointed out the correct place. The order is wrong there. I ran the E2E on the change, and it did pass 100%, however you might be right that there might be people out there relying on this. If we decide to make a PR for this, we should probably do it in a minor (or even a major). Until then you can just use pnpm patch or similiar if you need it.

[dirkdev98]

Thanks for confirming. Yes this ain't blocking, enough ways to work around it!

Let me know if you want this fixed in a minor, if so I can get a PR going.

[sommeeeer]

Let me know if you want this fixed in a minor, if so I can get a PR going.

That decision is up to Kheuzy or Conico. We'll have to wait and see what they want. Thanks for your research and willingnesss

[khuezy]

I'll have to defer to Conico here since, as I vaguely recall, he worked on the headers ordering per Nextjs' docs. Was that order always like that or did they change it?
If this is originally wrong, then we did a big whooops in the code review 😢

[conico974]

I recall testing this back at the time and it was the other way around, I could be wrong though, it's been awhile.
This is definitely a breaking change, and something i'd put on a major.
And since there is an easy workaround, i don't consider it an urgent matter. You could still create a PR and use the prerelease if you want, but we probably won't merge it right now.
We could also put it under a flag to enable this behavior (and this would not be a breaking change)

[sommeeeer]

We could also put it under a flag to enable this behavior (and this would not be a breaking change)

Thats actually an excellent idea.

[khuezy]

I recall testing this back at the time and it was the other way around

Same here, I recall us specifically discussing this somewhere (on github or on the discord)

[dirkdev98]

Thanks both for the quick replies! Will attempt to get a PR up later this week.