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: implement [build.upload.rules] for both service-worker/modules format workers #416

Closed
threepointone opened this issue Feb 8, 2022 · 7 comments · Fixed by #735, #756 or #721
Closed
Assignees
Labels
blocked Blocked on other work

Comments

@threepointone
Copy link
Contributor

threepointone commented Feb 8, 2022

One of wrangler2's highlights is having a first class module system, that brings it closer to runtimes like node/deno/etc. Further, we have pretty decent defaults ootb - common module types (js/jsx/ts/tsx/json) don't need any configuration, and non-"standard" modules (wasm/txt/html) work as expected. There are a few corner cases though, and a little more we can do to make this story feel more complete.

(This work is separate from the work for plugins (#367), which we'll get to later.)

@Electroid Electroid added this to the 2.0 milestone Feb 8, 2022
@Electroid Electroid moved this to Blocking in workers-sdk Feb 8, 2022
@threepointone threepointone self-assigned this Feb 25, 2022
@threepointone threepointone moved this from Must-have to In Progress in workers-sdk Feb 28, 2022
@threepointone
Copy link
Contributor Author

Most of this was done in #530, but some followups before we can close this

  • non-wasm modules don't work in local mode (I think I can fix this)
  • Data type does not work in service worker format, in either mode (because there isn't an analog to wasm_module/text_blob binding for a Data module, unless I'm missing something)

@threepointone
Copy link
Contributor Author

I proposed a partial fix for this in cloudflare/miniflare#205

threepointone added a commit that referenced this issue Mar 8, 2022
This rewrites how we pass configuration to miniflare in `wrangler dev`'s local mode. Instead of passing the entire configuration as cli args, we now generate a `wrangler.toml` based on our generated/inferred configuration, and pass that to miniflare instead. This solves a couple of issues, notably -

- `text_blobs` now works in local mode+service-worker format
- `Text` modules now work in local mode+service-worker format
- We properly throw errors for `Data` module in service-worker format

Along with cloudflare/miniflare#205, this fixes #416.
@threepointone threepointone moved this from Must-have to In Review in workers-sdk Mar 8, 2022
@threepointone threepointone self-assigned this Mar 8, 2022
threepointone added a commit that referenced this issue Mar 9, 2022
This rewrites how we pass configuration to miniflare in `wrangler dev`'s local mode. Instead of passing the entire configuration as cli args, we now generate a `wrangler.toml` based on our generated/inferred configuration, and pass that to miniflare instead. This solves a couple of issues, notably -

- `text_blobs` now works in local mode+service-worker format
- `Text` modules now work in local mode+service-worker format
- We properly throw errors for `Data` module in service-worker format

Along with cloudflare/miniflare#205, this fixes #416.
threepointone added a commit that referenced this issue Mar 9, 2022
This rewrites how we pass configuration to miniflare in `wrangler dev`'s local mode. Instead of passing the entire configuration as cli args, we now generate a `wrangler.toml` based on our generated/inferred configuration, and pass that to miniflare instead. This solves a couple of issues, notably -

- `text_blobs` now works in local mode+service-worker format
- `Text` modules now work in local mode+service-worker format
- We properly throw errors for `Data` module in service-worker format

Along with cloudflare/miniflare#205, this fixes #416.
@threepointone threepointone moved this from In Review to Must-have in workers-sdk Mar 9, 2022
@threepointone
Copy link
Contributor Author

for text_blobs/Text modules support in local mode, blocked on cloudflare/miniflare#211. After that, we can land #558 and close this. Unassigning from myself.

@threepointone threepointone removed their assignment Mar 9, 2022
@petebacondarwin petebacondarwin added the blocked Blocked on other work label Mar 15, 2022
@caass caass self-assigned this Mar 24, 2022
@caass
Copy link
Contributor

caass commented Mar 30, 2022

Opened cloudflare/miniflare#228 with more hope than tangible code

@threepointone
Copy link
Contributor Author

Deeply grateful to @caass for working on this with @mrbbot to land support in miniflare! Thanks thanks.

threepointone added a commit that referenced this issue Mar 31, 2022
This adds support for `text_blobs`/Text module support in local mode. Now that cloudflare/miniflare#228 has landed in miniflare (thanks @caass!), we can use that in wrangler as well.

Fixes #416
threepointone added a commit that referenced this issue Mar 31, 2022
This adds support for `text_blobs`/Text module support in local mode. Now that cloudflare/miniflare#228 has landed in miniflare (thanks @caass!), we can use that in wrangler as well.

Fixes #416
threepointone added a commit that referenced this issue Mar 31, 2022
This adds support for `text_blobs`/Text module support in local mode. Now that cloudflare/miniflare#228 has landed in miniflare (thanks @caass!), we can use that in wrangler as well.

Fixes #416
threepointone added a commit that referenced this issue Mar 31, 2022
This adds support for `text_blobs`/Text module support in local mode. Now that cloudflare/miniflare#228 has landed in miniflare (thanks @caass!), we can use that in wrangler as well.

Fixes #416
threepointone added a commit that referenced this issue Mar 31, 2022
…ode (#735)

This adds support for `text_blobs`/Text module support in local mode. Now that cloudflare/miniflare#228 has landed in miniflare (thanks @caass!), we can use that in wrangler as well.

Fixes #416
Repository owner moved this from Must-have to Done in workers-sdk Mar 31, 2022
@threepointone
Copy link
Contributor Author

reopening this, waiting for the miniflare update, as well as needing to add data_blob bindings (#740)

@threepointone threepointone reopened this Mar 31, 2022
Repository owner moved this from Done to In Progress in workers-sdk Mar 31, 2022
@threepointone
Copy link
Contributor Author

importing text and data modules now works in service workers+local mode, but not plain text_blobs/data_blobs. we probably have to copy it here https://github.com/cloudflare/wrangler2/blob/b9336414c3c1ac20ba34d274042886ea802385d9/packages/wrangler/src/dev/local.tsx#L75-L82 I'll fix this tomorrow unless someone else gets to it

threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths so that they're ersolveds correctly by miniflare.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths so that they're ersolveds correctly by miniflare.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as asbolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
threepointone added a commit that referenced this issue Apr 4, 2022
…#756)

For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes #740
Fixes #416
Repository owner moved this from In Progress to Done in workers-sdk Apr 4, 2022
mrbbot added a commit that referenced this issue Oct 31, 2023
)

- Upgrade to `ava@5`
- Port `KVNamespace` tests from Miniflare 2
- Only throw if `cacheTtl` is invalid, not just specified
- Return `cursor: undefined` on the last page to match the behaviour
  of the Worker's runtime
mrbbot added a commit that referenced this issue Nov 1, 2023
)

- Upgrade to `ava@5`
- Port `KVNamespace` tests from Miniflare 2
- Only throw if `cacheTtl` is invalid, not just specified
- Return `cursor: undefined` on the last page to match the behaviour
  of the Worker's runtime
mrbbot added a commit that referenced this issue Nov 1, 2023
)

- Upgrade to `ava@5`
- Port `KVNamespace` tests from Miniflare 2
- Only throw if `cacheTtl` is invalid, not just specified
- Return `cursor: undefined` on the last page to match the behaviour
  of the Worker's runtime
mrbbot added a commit that referenced this issue Nov 1, 2023
)

- Upgrade to `ava@5`
- Port `KVNamespace` tests from Miniflare 2
- Only throw if `cacheTtl` is invalid, not just specified
- Return `cursor: undefined` on the last page to match the behaviour
  of the Worker's runtime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment