feat: add support for the Frameworks API #6735
Conversation
| // The order of the directories matter: zip-it-and-ship-it will prioritize | ||
| // functions from the rightmost directories. In this case, we want user | ||
| // functions to take precedence over internal functions. | ||
| const functionDirectories = [internalFunctionsFolder, functionsFolder].filter((folder): folder is string => | ||
| Boolean(folder), | ||
| const functionDirectories = [internalFunctionsFolder, frameworksAPIPaths.functions.path, functionsFolder].filter( | ||
| (folder): folder is string => Boolean(folder), |
There was a problem hiding this comment.
Since the order is behaviourally important, would it be possible to add test coverage for it?
There was a problem hiding this comment.
We're already testing that here, by hitting the deployed site and asserting that the output comes from the function we expect. I can add a few more cases here in a follow-up, though.
| /** | ||
| * Returns an object containing the paths for all the operations of the | ||
| * Frameworks API. Each key maps to an object containing a `path` property | ||
| * with the path of the operation and a `ensureExists` methos that creates |
There was a problem hiding this comment.
nit:
| * with the path of the operation and a `ensureExists` methos that creates | |
| * with the path of the operation and an `ensureExists` method that creates |
| import { mkdir } from 'fs/promises' | ||
| import { resolve } from 'node:path' |
There was a problem hiding this comment.
consistency nit:
| import { mkdir } from 'fs/promises' | |
| import { resolve } from 'node:path' | |
| import { mkdir } from 'node:fs/promises' | |
| import { resolve } from 'node:path' |
| * the directory in case it doesn't exist. | ||
| */ | ||
| export const getFrameworksAPIPaths = (basePath: string, packagePath?: string) => { | ||
| const root = resolve(basePath, packagePath || '', '.netlify/v1') |
There was a problem hiding this comment.
TIL
> path.resolve('/hello', '', 'world')
'/hello/world'
There was a problem hiding this comment.
... huh, interesting rabbit hole:
> path.resolve('/hello', ' ', 'world')
'/hello/ /world'
> path.resolve(' ', ' ', '/hello', 'world')
'/hello/world'
> path.resolve('/hello', 'world', ' ')
'/hello/world/ '
| } | ||
|
|
||
| /** | ||
| * Returns an object containing the paths for all the operations of the |
| import { FunctionsRegistry } from '../../../../dist/lib/functions/registry.js' | ||
| import { watchDebounced } from '../../../../dist/utils/command-helpers.js' | ||
| import { getFrameworksAPIPaths } from '../../../../dist/utils/frameworks-api.js' |
There was a problem hiding this comment.
🤔 Huh, why do all the unit tests test the built code while the integration tests test the source? That's odd.
There was a problem hiding this comment.
Good question. I'm not sure! I know we've gradually introduced vitest over ava, and there was some complexity in that transition. But that doesn't explain this. I'll take a look.
| // The order of the function directories matters. Rightmost directories take | ||
| // precedence. |
There was a problem hiding this comment.
same here, is it possible to test this?
There was a problem hiding this comment.
Same answer as above. We end up testing this in the different commands by creating conflicting function and checking the output of the one that wins.
| @@ -23,6 +23,7 @@ export const getFeatureFlagsFromSiteInfo = (siteInfo: { | |||
| ...(siteInfo.feature_flags || {}), | |||
| // see https://github.com/netlify/pod-dev-foundations/issues/581#issuecomment-1731022753 | |||
| zisi_golang_use_al2: isFeatureFlagEnabled('cli_golang_use_al2', siteInfo), | |||
| netlify_build_frameworks_api: true, | |||
There was a problem hiding this comment.
🤔 Were you initially thinking of feature flagging this and then you changed your mind and forgot to remove this? I might be missing something otherwise.
There was a problem hiding this comment.
No, the idea here is to always enable the Frameworks API in Netlify Build, which is still behind a feature flag in our build system.
Summary
Adds support for the Frameworks API to the
deployandservecommands.