feat: use Frameworks API #2547
Draft
feat: use Frameworks API #2547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… able to use either previous setup with regional blobs or just legacy blobs
pieh
changed the title
📊 Package size report 0.06%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
commented
Jul 16, 2024
Comment on lines
+306
to
349
| export async function uploadBlobs(ctx: FixtureTestContext, pluginContext: PluginContext) { | ||
| if (pluginContext.blobsStrategy === 'frameworks-api') { | ||
| const files = await glob('**/blob', { | ||
| dot: true, | ||
| cwd: pluginContext.blobDir, | ||
| }) | ||
|
|
||
| const keys = files.filter((file) => !basename(file).startsWith('$')) | ||
| await Promise.all( | ||
| keys.map(async (key) => { | ||
| const { dir, base } = parse(key) | ||
| const metaFile = join(blobsDir, dir, `$${base}.json`) | ||
| const metadata = await readFile(metaFile, 'utf-8') | ||
| .then((meta) => JSON.parse(meta)) | ||
| .catch(() => ({})) | ||
| await ctx.blobStore.set(key, await readFile(join(blobsDir, key), 'utf-8'), { metadata }) | ||
| }), | ||
| ) | ||
| await Promise.all( | ||
| files.map(async (blobFilePath) => { | ||
| const { dir: key } = parse(blobFilePath) | ||
| const metaFile = join(pluginContext.blobDir, key, `blob.meta.json`) | ||
| const metadata = await readFile(metaFile, 'utf-8') | ||
| .then((meta) => JSON.parse(meta)) | ||
| .catch(() => ({})) | ||
| await ctx.blobStore.set( | ||
| key, | ||
| await readFile(join(pluginContext.blobDir, blobFilePath), 'utf-8'), | ||
| { | ||
| metadata, | ||
| }, | ||
| ) | ||
| }), | ||
| ) | ||
| } else { | ||
| const files = await glob('**/*', { | ||
| dot: true, | ||
| cwd: pluginContext.blobDir, | ||
| }) | ||
|
|
||
| const keys = files.filter((file) => !basename(file).startsWith('$')) | ||
| await Promise.all( | ||
| keys.map(async (key) => { | ||
| const { dir, base } = parse(key) | ||
| const metaFile = join(pluginContext.blobDir, dir, `$${base}.json`) | ||
| const metadata = await readFile(metaFile, 'utf-8') | ||
| .then((meta) => JSON.parse(meta)) | ||
| .catch(() => ({})) | ||
| await ctx.blobStore.set(key, await readFile(join(pluginContext.blobDir, key), 'utf-8'), { | ||
| metadata, | ||
| }) | ||
| }), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
This entire function doesn't seem like should be implemented here (as in in next-runtime repo) as it does reimplement file-based blobs uploads (something which ideally could be imported from ~netlify/build and just used) - now it "has to" support different implementations (when moving to regional blobs it was only directory change, but frameworks api also has updated directory structure).
This is not the biggest deal because it's quite straight forward - but maybe something to keep in mind in case ~integration kind of tests would be introduced in other repos to avoid duplicating something like that
pieh
commented
Jul 16, 2024
pieh
commented
Jul 16, 2024
| @@ -184,7 +184,7 @@ export async function runPluginStep( | |||
| // EDGE_FUNCTIONS_DIST: '.netlify/edge-functions-dist/', | |||
| // CACHE_DIR: '.netlify/cache', | |||
| // IS_LOCAL: true, | |||
| // NETLIFY_BUILD_VERSION: '29.23.4', | |||
| NETLIFY_BUILD_VERSION: '29.50.5', | |||
There was a problem hiding this comment.
This makes integration tests use Frameworks API code paths - because integration test setup rely on PluginContext already when looking for serverless and edge functions to bundle and invoke them, this almost doesn't need any additional changes other then adjustment to simulating file based blob uploads.
Is there value in trying to run integration tests vs multiple netlify/build version? I don't think it does given that we implement lot of integration/test setup ourselves, but doesn't hurt to ask for other opinions here.
I did add one ~kitchen-sink e2e test to run on ~older CLI implicitly testing blobs, serverless and edge functions as a way to at least have some ~smoke test to have some defense against regressions there in future, but it's pretty minimal
…765-migrate-next-runtime-to-use-frameworks-api
…765-migrate-next-runtime-to-use-frameworks-api
…765-migrate-next-runtime-to-use-frameworks-api
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
work in progress on migrating next runtime to use frameworks API. It still will support older method to support cases of CLI usage (which user might not updated)
Documentation
Tests
You can test this change yourself like so:
Relevant links (GitHub issues, etc.) or a picture of cute animal