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

feat: Frameworks API cleanup #5737

Merged
merged 12 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ packages/*/lib/
!**/fixtures/**/functions_*/.netlify
!**/fixtures/**/functions_*/.netlify/edge-functions/manifest.json
!**/fixtures/**/monorepo/**/.netlify
!**/tests/deploy_config/fixtures/**/.netlify
**/plugins/deno-cli

lib
2 changes: 1 addition & 1 deletion packages/build/src/plugins_core/edge_functions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const coreStep = async function ({

if (featureFlags.netlify_build_frameworks_api) {
if (await pathExists(frameworksAPISrcPath)) {
generatedFunctionPaths.unshift(frameworksAPISrcPath)
generatedFunctionPaths.push(frameworksAPISrcPath)
}

const frameworkImportMap = resolve(
Expand Down
2 changes: 1 addition & 1 deletion packages/build/src/plugins_core/functions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const zipFunctionsAndLogResults = async ({
// Printing an empty line before bundling output.
log(logs, '')

const sourceDirectories = [frameworkFunctionsSrc, internalFunctionsSrc, functionsSrc].filter(Boolean)
const sourceDirectories = [internalFunctionsSrc, frameworkFunctionsSrc, functionsSrc].filter(Boolean)
const results = await zipItAndShipIt.zipFunctions(sourceDirectories, functionsDist, zisiParameters)

validateCustomRoutes(results)
Expand Down
15 changes: 7 additions & 8 deletions packages/build/src/plugins_core/pre_cleanup/index.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
import { rm } from 'node:fs/promises'
import { resolve } from 'node:path'

import { scanForBlobs, getBlobsDirs } from '../../utils/blobs.js'
import { CoreStep, CoreStepCondition, CoreStepFunction } from '../types.js'
import { getBlobsDirs } from '../../utils/blobs.js'
import { FRAMEWORKS_API_ENDPOINT } from '../../utils/frameworks_api.js'
import { CoreStep, CoreStepFunction } from '../types.js'

const coreStep: CoreStepFunction = async ({ buildDir, packagePath }) => {
const blobsDirs = getBlobsDirs(buildDir, packagePath)
const dirs = [...getBlobsDirs(buildDir, packagePath), resolve(buildDir, packagePath || '', FRAMEWORKS_API_ENDPOINT)]

try {
await Promise.all(blobsDirs.map((dir) => rm(dir, { recursive: true, force: true })))
await Promise.all(dirs.map((dir) => rm(dir, { recursive: true, force: true })))
} catch {
// Ignore errors if it fails, we can continue anyway.
}

return {}
}

const blobsPresent: CoreStepCondition = async ({ buildDir, packagePath }) =>
Boolean(await scanForBlobs(buildDir, packagePath))

export const preCleanup: CoreStep = {
event: 'onPreBuild',
coreStep,
coreStepId: 'pre_cleanup',
coreStepName: 'Pre cleanup',
coreStepDescription: () => 'Cleaning up leftover files from previous builds',
condition: blobsPresent,
Copy link
Member Author

@eduardoboucas eduardoboucas Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that this core step will now run for all builds as opposed to only running if there are blobs directories. That is fine, since calling rm() on a directory that doesn't exist will not throw here, and we have a try/catch around the calls anyway.

quiet: true,
}
2 changes: 1 addition & 1 deletion packages/build/src/steps/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { DEV_EVENTS, EVENTS } from '../plugins/events.js'
import { uploadBlobs } from '../plugins_core/blobs_upload/index.js'
import { buildCommandCore } from '../plugins_core/build_command.js'
import { deploySite } from '../plugins_core/deploy/index.js'
import { applyDeployConfig } from '../plugins_core/deploy_config/index.js'
import { devUploadBlobs } from '../plugins_core/dev_blobs_upload/index.js'
import { bundleEdgeFunctions } from '../plugins_core/edge_functions/index.js'
import { applyDeployConfig } from '../plugins_core/frameworks_api/index.js'
import { bundleFunctions } from '../plugins_core/functions/index.js'
import { preCleanup } from '../plugins_core/pre_cleanup/index.js'
import { preDevCleanup } from '../plugins_core/pre_dev_cleanup/index.js'
Expand Down
9 changes: 3 additions & 6 deletions packages/build/src/utils/blobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,13 @@ const METADATA_SUFFIX = '.json'
* path to its metadata file.
*/
export const getKeysToUpload = async (blobsDir: string) => {
const blobsToUpload: { key: string; contentPath: string; metadataPath: string }[] = []
const files = await new fdir()
.withRelativePaths() // we want the relative path from the blobsDir
.filter((fpath) => !path.basename(fpath).startsWith(METADATA_PREFIX))
.crawl(blobsDir)
.withPromise()

files.forEach((filePath) => {
return files.map((filePath) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ❤️

const key = filePath.split(path.sep).join('/')
const contentPath = path.resolve(blobsDir, filePath)
const basename = path.basename(filePath)
Expand All @@ -124,14 +123,12 @@ export const getKeysToUpload = async (blobsDir: string) => {
`${METADATA_PREFIX}${basename}${METADATA_SUFFIX}`,
)

blobsToUpload.push({
return {
key,
contentPath,
metadataPath,
})
}
})

return blobsToUpload
}

/** Read a file and its metadata file from the blobs directory */
Expand Down
9 changes: 5 additions & 4 deletions packages/build/src/utils/frameworks_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { basename, dirname, resolve, sep } from 'node:path'

import { fdir } from 'fdir'

export const FRAMEWORKS_API_BLOBS_ENDPOINT = '.netlify/v1/blobs'
export const FRAMEWORKS_API_CONFIG_ENDPOINT = '.netlify/v1/config.json'
export const FRAMEWORKS_API_EDGE_FUNCTIONS_ENDPOINT = '.netlify/v1/edge-functions'
export const FRAMEWORKS_API_ENDPOINT = '.netlify/v1'
export const FRAMEWORKS_API_BLOBS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/blobs`
export const FRAMEWORKS_API_CONFIG_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/config.json`
export const FRAMEWORKS_API_EDGE_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/edge-functions`
export const FRAMEWORKS_API_EDGE_FUNCTIONS_IMPORT_MAP = 'import_map.json'
export const FRAMEWORKS_API_FUNCTIONS_ENDPOINT = '.netlify/v1/functions'
export const FRAMEWORKS_API_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/functions`

type DirectoryTreeFiles = Map<string, string[]>

Expand Down
24 changes: 12 additions & 12 deletions packages/build/tests/core/snapshots/tests.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -917,9 +917,9 @@ Generated by [AVA](https://avajs.dev).
Running \`netlify build\` will execute this build flow␊
┌────────────────────────────┐␊
│ Event │ Location │␊
└────────────────────────────┘␊
┌─────────────────┬─────────────────┐␊
│ Event │ Location │␊
└─────────────────┴─────────────────┘␊
If this looks good to you, run \`netlify build\` to execute the build␊
`
Expand Down Expand Up @@ -1026,9 +1026,9 @@ Generated by [AVA](https://avajs.dev).
Running \`netlify build\` will execute this build flow␊
┌────────────────────────────┐␊
│ Event │ Location │␊
└────────────────────────────┘␊
┌─────────────────┬─────────────────┐␊
│ Event │ Location │␊
└─────────────────┴─────────────────┘␊
If this looks good to you, run \`netlify build\` to execute the build␊
`
Expand Down Expand Up @@ -1073,12 +1073,12 @@ Generated by [AVA](https://avajs.dev).
Running \`netlify build\` will execute this build flow␊
┌────────────────────────────┐␊
│ Event │ Location │␊
└────────────────────────────┘␊
┌──────────────┐␊
│ 1. onBuild ↓ │ Build command from Netlify app␊
└──────────────┘ ␊
┌─────────────────┬─────────────────┐␊
│ Event │ Location │␊
└─────────────────┴─────────────────┘␊
┌─────────────────┐␊
│ 1. onBuild ↓ │ Build command from Netlify app␊
└─────────────────┘ ␊
If this looks good to you, run \`netlify build\` to execute the build␊
`
Expand Down
Binary file modified packages/build/tests/core/snapshots/tests.js.snap
Binary file not shown.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { cp } from 'node:fs/promises'
import { resolve } from "node:path"

await cp(resolve("frameworks_api_seed"), resolve(".netlify/v1"), { recursive: true })
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { greeting } from "greeting"

export default async () => new Response(greeting)

export const config = {
excludedPath: "/framework/skip_*",
generator: "Hello",
path: "/framework/*"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"imports": {
"greeting": "./util/greeting.ts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const greeting = "Hello world"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build]
command = "node build.mjs"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated in the legacy internal directory")

export const config = {
path: "/frameworks-internal-conflict/internal"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated by the Frameworks API")

export const config = {
path: "/frameworks-internal-conflict/frameworks"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated by the Frameworks API")

export const config = {
path: "/frameworks-user-conflict/frameworks"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { cp } from 'node:fs/promises'
import { resolve } from "node:path"

await cp(resolve("frameworks_api_seed"), resolve(".netlify/v1"), { recursive: true })
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated by the Frameworks API")

export const config = {
path: "/frameworks-internal-conflict/frameworks"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated by the Frameworks API")

export const config = {
path: "/frameworks-user-conflict/frameworks"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { greeting } from "greeting"

export default async () => new Response(greeting)

export const config = {
excludedPath: "/framework/skip_*",
generator: "Hello",
path: "/framework/*"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"imports": {
"greeting": "./util/greeting.ts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const greeting = "Hello world"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build]
command = "node build.mjs"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default async () => new Response("Generated by the user")

export const config = {
path: "/frameworks-user-conflict/user"
}
22 changes: 20 additions & 2 deletions packages/build/tests/edge_functions/snapshots/tests.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -767,11 +767,18 @@ Generated by [AVA](https://avajs.dev).
packages/build/tests/edge_functions/fixtures/functions_user_framework␊
> Config file␊
No config file was defined: using default values.
packages/build/tests/edge_functions/fixtures/functions_user_framework/netlify.toml
> Context␊
production␊
build.command from netlify.toml ␊
────────────────────────────────────────────────────────────────␊
$ node build.mjs␊
(build.command completed in 1ms)␊
Edge Functions bundling ␊
────────────────────────────────────────────────────────────────␊
Expand Down Expand Up @@ -806,21 +813,32 @@ Generated by [AVA](https://avajs.dev).
packages/build/tests/edge_functions/fixtures/functions_user_internal_framework␊
> Config file␊
No config file was defined: using default values.
packages/build/tests/edge_functions/fixtures/functions_user_internal_framework/netlify.toml
> Context␊
production␊
build.command from netlify.toml ␊
────────────────────────────────────────────────────────────────␊
$ node build.mjs␊
(build.command completed in 1ms)␊
Edge Functions bundling ␊
────────────────────────────────────────────────────────────────␊
Packaging Edge Functions from .netlify/edge-functions directory:␊
- frameworks-internal-conflict␊
- function-3␊
Packaging Edge Functions generated by your framework:␊
- frameworks-internal-conflict␊
- frameworks-user-conflict␊
- function-2␊
Packaging Edge Functions from netlify/edge-functions directory:␊
- frameworks-user-conflict␊
- function-1␊
(Edge Functions bundling completed in 1ms)␊
Expand Down
Binary file modified packages/build/tests/edge_functions/snapshots/tests.js.snap
Binary file not shown.
Loading
Loading