- 
                Notifications
    You must be signed in to change notification settings 
- Fork 431
fix: bundle edge functions if they exist on deploy --no-build #7743
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
Conversation
236579a    to
    1690d6d      
    Compare
  
    …nd use inline config, also add cases for internal and framework functions
1690d6d    to
    dabc560      
    Compare
  
    5602c96    to
    4fe4444      
    Compare
  
    | // (cachedConfig type error hides this one, but it still is valid) @ts-expect-error FIXME(serhalp): This is missing from the `runCoreSteps` type in @netlify/build | ||
| edgeFunctionsBootstrapURL: await getBootstrapURL(), | ||
| // @ts-expect-error 'CachedConfig' is not assignable to type 'Record<string, unknown>'. | ||
| // Index signature for type 'string' is missing in type 'CachedConfig'. | ||
| cachedConfig: command.netlify.cachedConfig, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bit frustrating, once I do add cachedConfig here, it no longer complain about edgeFunctionsBootstrapURL and now it actually complain about @ts-expect-error for it ... but this is still valid, just somehow hidden because of cachedConfig type error ...
Passing cachedConfig seems needed to properly support --cwd other/dir case, as otherwise bundling was trying to look in current directory and there doesn't seem to be other arguments we can pass to tell it to look elsewhere (despite passing cwd explitly above).
| // build flag wasn't used and edge functions directories exist | ||
| if (!options.build && (await anyEdgeFunctionsDirectoryExists(command))) { | ||
| // for the case of directories existing but not containing any edge functions, | ||
| // there is early bail in edge functions bundling after scanning for edge functions | ||
| // for this case and to avoid replicating scanning logic here, we defer to the bundling step | ||
| await bundleEdgeFunctions(options, command) | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous condition doesn't seem to be met unless you specify some edge functions configuration in toml (?) so instead we check if there are edge functions dir present and trigger on-demand edge bundling if they are
if directories exist but contain on edge functions - there will be early bail - that's just edge bundling function step in @netlify/build works, so I opted not trying to repeat same logic here
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes https://linear.app/netlify/issue/FRB-2024/ensure-edge-functions-are-bundled-with-no-build-option
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)