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: Workers Sites support for local mode #336

Merged
merged 4 commits into from
Jan 29, 2022
Merged

feat: Workers Sites support for local mode #336

merged 4 commits into from
Jan 29, 2022

Conversation

threepointone
Copy link
Contributor

There are 3 commits in this PR.

  • One that adds a Workers Sites example, example-sites-app.
  • Another that inlines text files into the Worker bundle directly (instead of adding them as 'modules' in the Worker definition)
  • And another that implements the actual feature, where Workers Sites now works in local mode when running wrangler dev.

There are some bug fixes -

  • fixes a bug where we were sending the __STATIC_CONTENT_MANIFEST definition as a separate module even with service worker format,
  • fixes a bug where we weren't uploading the static content namespace binding when other kv namespaces weren't present.
  • fixes an infinite fetching loop of KV namespace keys (also replicated in fix: prevent infinite loop when fetching a list of results #335, which will remove it from here after it lands)

@changeset-bot
Copy link

changeset-bot bot commented Jan 29, 2022

🦋 Changeset detected

Latest commit: 03dd897

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I wondered whether the first commit needed a changeset. But then I realised that this will never be published, it is more of an example, than a package. So that made me think: perhaps example-worker-app and this new example-sites-app should not be in the packages directory at all??

Please can you add tests for the fixes to publish since that is possible right now. The dev fix is not so easy to test yet, so we can let that pass for now.

packages/wrangler/src/cfetch/index.ts Outdated Show resolved Hide resolved
packages/wrangler/src/dev.tsx Outdated Show resolved Hide resolved
An example app with a Workers Sites definition.
We were adding text-like modules (i.e. `.txt`, `.html` and `.pem` files) as separate modules in the Worker definition, but this only really 'works' with the ES module Worker format. This commit changes that to inline the text-like files into the Worker bundle directly.

We still have to do something similar with `.wasm` modules, but that requires a different fix, and we'll do so in a subsequent commit.
This adds support for Workers Sites in local mode when running wrangler `dev`. Further, it fixes a bug where we were sending the `__STATIC_CONTENT_MANIFEST` definition as a separate module even with service worker format, and a bug where we weren't uploading the namespace binding when other kv namespaces weren't present.
packages/wrangler/src/api/form_data.ts Outdated Show resolved Hide resolved
packages/wrangler/src/dev.tsx Show resolved Hide resolved
@threepointone threepointone merged commit ce61000 into main Jan 29, 2022
@threepointone threepointone deleted the local-sites branch January 29, 2022 20:34
@github-actions github-actions bot mentioned this pull request Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants