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

Upload multiple module workers #1128

Open
lunaris opened this issue Nov 17, 2022 · 5 comments
Open

Upload multiple module workers #1128

lunaris opened this issue Nov 17, 2022 · 5 comments

Comments

@lunaris
Copy link

lunaris commented Nov 17, 2022

Current cloudflare-go version

Latest

Description

It would be great to be able to upload ESM workers which have multiple modules (e.g. those with .wasm dependencies). As best as I can tell this is supported by the existing API using multipart form data (which is what wrangler is using to support this use case), so could be also be supported by cloudflare-go. I am happy to implement the feature/raise a PR but thought it might be good to discuss the design/whether this is something that would be accepted beforehand.

Currently the interface for uploading a worker script is captured by the WorkerScriptParams type:

// WorkerScriptParams provides a worker script and the associated bindings.
type WorkerScriptParams struct {
	Script string

	// Module changes the Content-Type header to specify the script is an
	// ES Module syntax script.
	Module bool

	// Bindings should be a map where the keys are the binding name, and the
	// values are the binding content
	Bindings map[string]WorkerBinding
}

At present the Script field of this struct is the only way to pass module code to be uploaded. I can think of a few options to extend this to support multiple-module use cases:

  • Change Script to Scripts, allowing requests to take multiple scripts. This would require us to add some validation to ensure that at least one script is passed, and likely some checks to prevent Module = true with multiple scripts. Scripts[0] would be the main worker script. A bit ugly, perhaps.
  • Add a new field, AdditionalScripts (better names welcome) that captures additional scripts, with Script becoming the main worker script. Again, we'd likely want to make sure that Module = true if this new field is not empty.
  • My particular use case is .wasm files. The CommonJS API already supports WASM bindings, but rejects them if Module = true. We could modify the code to move WASM bindings to the multi-part body if Module = true. This would not require us to change the function API, but could lead to unexpected behaviour/be a bad developer experience.

We might need to tweak the Download* functions also, though I understand some work has gone into making these work with multipart workers already (though they may only be expecting a single part).

Thoughts, critique and other feedback welcome on this -- it'd be great to bring cloudflare-go to parity with wrangler so that we can expose this functionality neatly in e.g. Terraform and Pulumi codebases.

Use cases

Supporting multi-module workers in cloudflare-go would lead to this support being available in Terraform/Pulumi, allowing infrastructure-as-code setups that don't depend on running wrangler commands.

Potential cloudflare-go usage

// WorkerScriptParams provides a worker script and the associated bindings.
type WorkerScriptParams struct {
	Script string

	// Additional modules that should be uploaded as part of the script
	AdditionalScripts string[]

	// Module changes the Content-Type header to specify the script is an
	// ES Module syntax script.
	Module bool

	// Bindings should be a map where the keys are the binding name, and the
	// values are the binding content
	Bindings map[string]WorkerBinding
}

References

No response

@jacobbednarz
Copy link
Member

jacobbednarz commented Nov 18, 2022

thanks @lunaris, this is a good start. you're largely correct that we primarily target single script workers and some WASM functionality is lacking.

i would hold off on any changes for now as i'm about to revamp the workers files (like i did with workers KV in #1115) to drop some extra bits we no longer need. if you can wait until after that, not only will it be cleaner but we'll be using the newer method signatures that rely on structs instead of many params.

@jacobbednarz
Copy link
Member

fwiw, i don't think we'll ever have full parity with wrangler as wrangler is focused on the entire ecosystem experience (bundling, local dev, etc) whereas Terraform is only about the lifecycle management of resources.

@omarabid
Copy link

@lunaris @jacobbednarz Is it possible to bundle the .js+.wasm file into a single blob and pass it as content for the worker? It is possible to import the worker through Terraform. Its content shows up as both JS+Wasm somehow mixed up.

@tonysangha
Copy link

Hey @jacobbednarz, Is there an update on this issue?

@jacobbednarz
Copy link
Member

no updates from me other than the work i mentioned was in #1128 (comment) completed in #1137.

if this is something you need, i'd recommend getting in touch with the Workers team to drop this in their backlog for prioritisation.

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

No branches or pull requests

4 participants