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

RFC: Static Assets with Workers #1162

Closed
threepointone opened this issue Jun 1, 2022 · 15 comments
Closed

RFC: Static Assets with Workers #1162

threepointone opened this issue Jun 1, 2022 · 15 comments
Assignees
Labels
discussion This issue is being used to discuss a topic rather than track a feature or bug.

Comments

@threepointone
Copy link
Contributor

threepointone commented Jun 1, 2022

This one is pretty straightforward. As a website developer, I would like to serve "static" assets on the internet. These are not user generated content, but usually stuff like html file, logos, maybe some images, etc.

The current options are:

  • Workers Sites, which requires a combination of configuration ([site] in wrangler.toml) and code (configuring @cloudflare/kv-asset-handler). It's incredibly fragile and not particularly liked by the community. It's backed by KV. It also has issues with consistency since there's no efficient way to expire assets, so during deploy it's possible that some assets just 404. It was the only thing folks had for a while.
    • We should silently stop supporting this. We shouldn't add a deprecation warning, and @cloudflare/kv-asset-handler should still get security updates and bugfixes, but we shouldn't add any new features, and guide people towards newer solutions.
  • Pages, which is an opinionated stack around Workers. Also backed by KV, but the implementation is not exposed to the user. They've announced a direct upload api, but it's still not clear how it would integrate with a wrangler project.
    • If it's a new project that's centered around static assets, the recommendation is to simply start with Pages.
    • It's hard to migrate an existing non-trivial project to use Pages (mostly because of missing features).
    • We can't currently recommend people just use Pages for static assets until Pages specifies how that would work. (I... don't know myself? How would it work? Does it require the convergence effort? How would routing work? And references? And previews? I suspect we'll use the upload api as an implementation detail underneath an abstraction, possibly one of the below options)
  • --experimental-public <path>: Exists in wrangler2. Backed by KV and kv-asset-handler, but the implementation is not exposed to the user. Light on features. Faces the same 404 issue as Sites. Particularly nice because it's "zero config"

Plan 1: --assets <path>

  • Rename --experimental-public to --assets
  • Allow multiple values (--assets <path1> --assets <path2>)
  • Add to wrangler.toml as assets
  • Optionally have an object form { bucket: <bucket>, include: <glob>, ... }
    • bucket: path to assets
    • include: glob that matches files to include
    • exclude: glob that matches files to exclude
    • mount: path to mount to
  • Implementation-wise, this can stick to using KV for now, but
    • We should move to using R2 to get past the 25 mb limit
    • We should start work on static-assets-as-a-cloudflare-primitive
    • We might use the Pages upload api for this
    • Gives us a path to solve the 404/consistency issues

Plan 2: imports

import ASSETS from "assets:path/to/bucket?include=glob&exclude=glob";

ASSETS.fetch(request, env); // env is required because it'll contain the kv (or whatever) binding 
  • This is the story for people who'll want deep customisation on how they want to handle static assets.
  • With code/abstraction, they'll be able to support usecases like those described vy the options in http://expressjs.com/en/resources/middleware/serve-static.html
  • There's a story here where we can graduate this to a proper web standard
  • It's critical that we figure out how to make this work with jest etc

Plan 3: File loader

Adds a new type to rules config

[[rules]]
type = "file"
globs = "glob/*.png"
import styles from style.css;

console.log(styles);
// http://something.com/path/to/style.css
  • This is useful for serving single files
  • Further useful to use as references (href/src) in html
  • For files below a particular size, we could embed them directly in the Worker (with data: uris, but this requires the WOrkers runtime to support this as well)

Plan 4: Integrate Pages

We should come up with a plan to integrate Pages for assets directly into a wrangler project

Open questions:

  • Should we use Pages upload API?
  • R2?
  • What's a good solution for expiring assets?
  • With versioning, we won't be able to 'delete' assets like we now do.
  • How can we hide the kv namespaces we make?
  • Overall, is there a cloudflare story for first class static assets?
@threepointone threepointone added the discussion This issue is being used to discuss a topic rather than track a feature or bug. label Jun 1, 2022
@threepointone threepointone self-assigned this Jun 1, 2022
@threepointone threepointone moved this to Untriaged in workers-sdk Jun 1, 2022
@mangs
Copy link

mangs commented Jun 1, 2022

Thank you for addressing this. I've been struggling with this while trying to figure out how to use the platform with a module-based worker despite the documentation around doing so not being 100%. It's more complicated than I would hope for (it's a simple, public repo if you're interested), but I love what you're doing as a company so I'm happy to help.

As members of the WinterCG, is your intent to consider what Deno Deploy is doing here too, or are you just collecting ideas? I don't want to clutter the thread up with text that is unwarranted.

@threepointone
Copy link
Contributor Author

Yeah I personally feel the pain when using Workers Sites, so I've got my own motivation for making this dead simple to use :)

I definitely want this to become a discussion among runtimes. Specifically, with Plan 2 (imports), the idea is that it (or whatever it becomes) would be the interface for developers who wish to serve static files, and Deno's serve/ express.static / Workers Sites / etc would become the implementation for how it works. I'd be happy with ideas for the interface itself (indeed, using this custom protocol asset: is clearly not going to be the final solution, I suspect it'll make use of import assertions/attribute evaluators/transforms)

@threepointone
Copy link
Contributor Author

threepointone commented Jun 1, 2022

btw, have you tried --experimental-public? The usage is relatively simple (here's an example https://github.com/threepointone/react-lazy-ssr-workers/blob/c62d7f4673b759daeb9591f34c8d991555eb0301/package.json#L8-L9) but it doesn't have any include/exclude config yet. Note the complete lack of any additional code in the worker itself!

Happy to see your repo too! Is it this one? https://github.com/mangs/homepage-solid This seems like a good fit.

@mangs
Copy link

mangs commented Jun 1, 2022

That's the one! Thank you for taking a look. I'll try that out and report back.

As for the topic proposal, it seems like some combination of Plan 1 (to actually upload the assets using wrangler) and Plan 2 (to optionally handle the assets) will be required.

However, I'd like to propose a Plan 5 to better enable isomorphic applications built on edge-rendered platforms when exact asset paths are known ahead of time. Plan 2 uses a Webpack-like "magic" string to create a fetching function, and considering your interest in potentially standardizing this functionality got me thinking of the Import Assertions and CSS Modules standard proposals which work similarly. Supporting those directly would be really helpful especially for the inevitably increasing number of folks who will learn about import assertions on the frontend; offering web-first compat here could be an easy onramp for new users.

Perhaps Plan 2 could benefit from import assertions too to remove the Webpack-like query params and instead offer more direct key-value configuration parameters. Thinking out loud here:

import { fetchAssets } from "path/to/bucket" assert { type: "assets", include: 'glob', exclude: 'glob };

using fetchAssets() instead of fetch() to prevent overusing the term "fetch". Already, workers use it as an export, and it's used as the web-compatible async fetching library, so adding another fetch() seems like one too many.

Curious of your thoughts.

EDIT: I just re-read your first reply; dunno how I missed your mention of import assertions 🙃

@mangs
Copy link

mangs commented Jun 3, 2022

I've been messing around with --experimental-public and I really like the simplicity. The main thing I miss is the ability to control the response (e.g. cache headers) that I was able to do before. The asset requests don't even reach my worker anymore, so I assume there's a behind the scenes worker somewhere else handling them. Also, I can no longer use --local; I get an error when doing so.

@threepointone
Copy link
Contributor Author

Hence the experimental prefix haha.

@mangs
Copy link

mangs commented Jun 3, 2022

Understood, I'll keep my eye on this as it develops. Thank you for sharing this new feature!

@petebacondarwin petebacondarwin moved this from Untriaged to Backlog in workers-sdk Jun 6, 2022
@GregBrimble
Copy link
Member

We've talked about it before, but just so it's documented here, I think my vote would be for an option 2 and 4 combo.

Importing

When Pages Plugins launched, we also launched static asset importing that looks like this:

export { onRequest } from 'assets:../public';

It would be pretty easy to also support a regular fetch on this too.

Serving

Serving like Pages does means you adopt our routing conventions (e.g. /foo/index.html is served at /foo/, and possibly support _headers and _redirects). We could be responsible for maintaining this (we want to open-source this anyway).

Local dev would just be done by pointing to your folder, but when you actually publish, you could essentially run wrangler pages publish on that static asset directory to create a deployment, and stitch it up with fetch('https://<hash?>.<project>.pages.dev/${pathname}') glue. This would also mean you don't need to consider expiring assets or caching.

We could have a Pages project for:

  1. each folder that gets uploaded,
  2. each Worker service (and if there are multiple static asset folders in that service, interleaving deployments), or
  3. a single project (with all static asset folder deployments interleaved)

The one concern about 2 & 3 is that we've recently had to demote the performance of non-aliased traffic (i.e. deployments that you directly access at hash.project.pages.dev. I believe branch aliases and production traffic are unaffected.

We could explore hiding these projects from a user in the dash if that made sense.

@threepointone
Copy link
Contributor Author

Potentially related https://github.com/tc39/proposal-import-reflection

@mangs
Copy link

mangs commented Jun 9, 2022

That's really interesting. Similar to the Twitter feedback so far, I'm surprised it's a string and not following the options object formatting like import assertions and CSS modules. Luca makes a good point that it's still early stages and they're collecting feedback about its use, but I'm still surprised they didn't start there to align with the existing drafts that are farther along. Seemingly oddly enough, that's how dynamic imports work in the proposal. I'm interested to learn more.

threepointone added a commit that referenced this issue Jun 12, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets` (and adds `--assets-include and `--assets-exclude` for parity with Sites.

Via #1162
threepointone added a commit that referenced this issue Jun 12, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets` (and adds `--assets-include and `--assets-exclude` for parity with Sites.

Via #1162
@rozenmd
Copy link
Contributor

rozenmd commented Jun 14, 2022

I was just thinking out loud how it's cool how it's almost possible to deploy a static eleventy site in 3 commands:

echo '# Page header' > README.md
npx @11ty/eleventy
npx wrangler publish --assets _site 

I'm thinking if an assets flag is provided, that should be enough for us to provide a boilerplate worker, and run dev/deploy with no further config required.

Folks can of course provide their own worker script if they'd like, but I'm thinking it shouldn't be a requirement any more.

threepointone added a commit that referenced this issue Jun 16, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets` (and adds `--assets-include and `--assets-exclude` for parity with Sites.

Via #1162
threepointone added a commit that referenced this issue Jun 16, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
threepointone added a commit that referenced this issue Jun 16, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
threepointone added a commit that referenced this issue Jun 16, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
threepointone added a commit that referenced this issue Jun 17, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
threepointone added a commit that referenced this issue Jun 17, 2022
This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
threepointone added a commit that referenced this issue Jun 17, 2022
…#1237)

This adds support for defining `assets` in `wrangler.toml`. You can configure it with a string path, or a `{bucket, include, exclude}` object (much like `[site]`). This also renames the `--experimental-public` arg as `--assets`.

Via #1162
@Vladi-ed
Copy link

Vladi-ed commented Jul 3, 2022

It's not clear what's the difference between
wrangler pages publish ./_site --project-name site
and
wrangler publish --assets _site

@mangs
Copy link

mangs commented Jul 3, 2022

@Vladi-ed wrangler pages is specifically for Cloudflare Pages. wrangler publish is for Cloudflare Workers.

@penalosa
Copy link
Contributor

Closing this for now since wrangler publish has support for the --assets flag.

@github-project-automation github-project-automation bot moved this from Backlog to Done in workers-sdk Feb 13, 2023
@threepointone
Copy link
Contributor Author

@penalosa did you remove the experimental warning? What is the decision on the rest of the action points?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is being used to discuss a topic rather than track a feature or bug.
Projects
None yet
Development

No branches or pull requests

6 participants