Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open Next Config validation #646

Open
vicb opened this issue Nov 25, 2024 · 1 comment
Open

Open Next Config validation #646

vicb opened this issue Nov 25, 2024 · 1 comment

Comments

@vicb
Copy link
Contributor

vicb commented Nov 25, 2024

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
Copy link
Contributor

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

const defaultOverrides = {
wrapper: "aws-lambda",
converter: "aws-apigw-v2",
tagCache: "dynamodb",
queue: "sqs",
incrementalCache: "s3",
imageLoader: "s3",
originResolver: "pattern-env",
warmer: "aws-lambda",
proxyExternalRequest: "node",
};

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants