Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

Not compatible with the new ES6 module worker format #174

Closed
PeterCxy opened this issue Apr 7, 2021 · 9 comments
Closed

Not compatible with the new ES6 module worker format #174

PeterCxy opened this issue Apr 7, 2021 · 9 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@PeterCxy
Copy link

PeterCxy commented Apr 7, 2021

It seems that this package still has the assumption of the old worker format (e.g. global environment variables, FetchEvent, etc) and does not work properly with the new ES6 module format. I would like to use Durable Objects, which requires the new ES6 module format, in a Workers Site project, and wrangler already has a patchset for Workers Site support with the new ES6 module format, and the fact that this package does not support the new format seems to be the last blocker.

@frandiox
Copy link

Same here, trying to implement the new custom builds in Vitedge but all calls to getAssetFromKV throw 500 internal errors.

@kristianfreeman
Copy link
Contributor

wrangler already has a patchset for Workers Site support with the new ES6 module format

can you link me to this?

Tagged this as help wanted - we're going to work on a release this week that probably won't include this unless someone would like to contribute a PR, but I'll mark this as next up for the release after that one 👍

@ttraenkler
Copy link
Contributor

@signalnerve i've patched the immediate roadblocker in the above PR #188 - please have a look if that's a suitable fix.

@ttraenkler
Copy link
Contributor

Note I merely patched the crash blocking us, but a proper solution would probably pass the env object as a third parameter or similar. however i didn't want to mix this fix with a necessary discussion about how to change the public API going forward. would be nice if this could be released as a fix asap, at least in some esm branch on npm as it complicates the build process for us.

@Cherry
Copy link
Contributor

Cherry commented Jun 1, 2021

Related wrangler issue: cloudflare/wrangler-legacy#1938

@DrewRidley
Copy link

For one method, getAssetFromKV, the fix appears quite trivial. It should be as simple as creating an override that accepts arguements, request, options, context as opposed to event. Since the only two properties of event used were waitUntil and request, they can be substituted with request and context.waitUntil instead. This would improve compatibility with ES6 module workers. I am happy to submit a PR with this change, but I am unfamiliar with the PR process for this repository.

@DrewRidley
Copy link

Please have a look at my PR, I introduce a fix for this problem. Sadly, the following problem will still cause problems when you try to deploy your site:
cloudflare/wrangler-legacy#1938.
I looked into that issue but could not diagnose the issue. Regardless, this should not have an impact on the ability to merge my PR.

@threepointone
Copy link
Contributor

We're cutting a release on both wrangler and kv-asset-handler that should provide a path forward. I'll send a PR documenting how very soon.

@threepointone
Copy link
Contributor

released, with the readme updated, we'll update the official docs as well soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants