-
Notifications
You must be signed in to change notification settings - Fork 87
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: add blob storage #2287
feat: add blob storage #2287
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for netlify-plugin-nextjs-demo-all-flags ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
packages/runtime/blob-cjs-build.mjs
Outdated
import * as esbuild from '@netlify/esbuild' | ||
|
||
// https://github.com/evanw/esbuild/issues/1760#issuecomment-964900401 | ||
const stripNodeColonPlugin = { | ||
name: 'strip-node-colon', | ||
setup({ onResolve, onLoad }) { | ||
onResolve({ filter: /^node:/ }, (args) => { | ||
return { path: args.path.slice('node:'.length), external: true } | ||
}) | ||
}, | ||
} | ||
|
||
await esbuild.build({ | ||
entryPoints: ['src/templates/netliblob.mts'], | ||
format: 'cjs', | ||
bundle: true, | ||
outfile: 'src/templates/blob.js', | ||
platform: 'node', | ||
plugins: [stripNodeColonPlugin], | ||
}) |
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 compiles ESM-only @netlify/blobs
to CJS and removes node:
prefixes in import/requires (like from node:fs
) to make resulting code compatible with our minimal node version
setBlobInit({ | ||
authentication: { | ||
contextURL: data.url, | ||
token: data.token, | ||
}, | ||
context: `deploy:${event.headers['x-nf-deploy-id']}`, | ||
siteID: event.headers['x-nf-site-id'], | ||
}) |
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.
We use Blob Storage in a place that doesn't have direct access to event
/context
which is needed to initialize the store, so instead we globally set init argument on each request. This means it's possible that requests would use init data we got from another one (depending on timing), but this is acceptable/ok to do and only note is that there is expiration time on token so that's why this being refreshed on each request and not just once.
if (!globalThis.fetch) { | ||
const fetch = require('node-fetch') | ||
|
||
globalThis.fetch = fetch | ||
globalThis.Headers = fetch.Headers | ||
globalThis.Request = fetch.Request | ||
globalThis.Response = fetch.Response | ||
} |
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 used just in Next's e2e tests setup, everywhere else there is global fetch (@netlify/blobs
require fetch
to be either passed in or to be globally available)
/** | ||
* @netlify/blobs ATM has some limitation to keys, so we need to normalize it for now (they will be resolved so we will be able to remove this code) | ||
*/ | ||
export const getNormalizedBlobKey = (key: string): string => Buffer.from(key).toString('base64url') |
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.
Is this fixed now? I know soemthing about encoding keys has landed
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.
It's not yet fully fixed - some fixes did land, but for now result is that keys starting with /
are no longer throwing, but trying to read those keys result in null
values, so we still need this for time being
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.
Wow, this was quite an undertaking! Thanks @pieh, looks great.
// `fallback: true` is not working correctly with ODBs | ||
// as we will cache fallback html forever, so | ||
// we are treating those as `fallback: blocking` | ||
// by editing the manifest |
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.
Thanks for documenting all these!
I still get notified since I opened the PR. Great work @pieh! |
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.
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.
All looks dandy 🚢
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.
Description
Uses Netlify blob storage for prerendered content instead of storing prerendered content in the generated lambdas, assuming blob storage is configured for the site.
There are 2 distinct and somewhat separate changes here:
x-prerender-revalidate
header to force revalidation) - this applies also when blob storage is not used (then it will use files from lambda instead of blob storage)note here - this change makes
fallback: true
pages behavior really bad, because ODB would cache those fallback (think Skeleton) so for users they would always first see skeleton page before browser client would re-render with actual content (if navigated to a page directly, and not using client navigation like with@next/link
).So to address that this also makes
fallback: true
behave more likefallback: blocking
(which is behavior we do have today, so we are not changing it really)@netlify/blobs
to CJS viabuild:blob
script - runtime is CJS and although there are tricks and hacks to still be able to use ESM version there are a lot of problems with them (await import
is compiled torequire
using our currenttsconfig
, anyeval
tricks to avoid that later make lambda bundling problematic etc)next-runtime-use-blobs-for-isr-assets
) and if that is enabled we also test for availability of blob store to determine wether to use it (there are no additional feature flags)fs.readFIle
will download a file from blob store (if the file to read was uploaded at build time). We already had mechanism like that that would pull files from CDN for SSG assets (like for examplenext-server
deciding to serve 404 page) - that mechanism now can also pull from blob store as well.Documentation
Tests
You can test this change yourself like so:
Relevant links (GitHub issues, etc.) or a picture of cute animal
Fixes
Fixes https://github.com/netlify/pod-ecosystem-frameworks/issues/568
Fixes FRA-34