opennextjs / opennextjs-aws

Open-source Next.js adapter for AWS
https://opennext.js.org
MIT License
4.41k stars 131 forks source link

Open Next Config validation #646

Open vicb opened 1 day ago

vicb commented 1 day ago

See:

I was looking at improving the configuration validation after the comment in #642

However I think a prerequisite would be to refactor the code before we do the validation:

Maybe we should: 1) Resolve the Open Next config early - that is fill in the defaults when no provided 2) Used the resolved config for validation

@conico974, you have much more experience than me with the codebase, what do you think?

conico974 commented 1 day ago

I think the main issue here is that we are mixing the edge runtime from Next, and the edge runtime from cloudflare.

My take on this is that we should only use edge for the Next edge runtime and find another name for cloudflare (or other) running runtime.

As for the prerequisite that you mentionned an easy fix would be to add an isEdgeRuntime(the Next one) param in the resolve plugin and apply different set of default overrides in this case instead of only the default one here https://github.com/opennextjs/opennextjs-aws/blob/f685ddea8f8a5c82591dc02713aff7138f2d9896/packages/open-next/src/plugins/resolve.ts#L57-L67

We should never rely on the name btw (that's my bad) as it could be a custom override.

Maybe we should:

  1. Resolve the Open Next config early - that is fill in the defaults when no provided
  2. Used the resolved config for validation

That's a good idea, but this will need to be done on a per function basis because each one could use very different overrides. We could compute some kind of OverrideObject that would contain everything we need

We can merge #642 and handle all of this in a different PR