-
Notifications
You must be signed in to change notification settings - Fork 79
feat: update edge functions bundling to Deno 2.x #6716
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
Conversation
| @@ -0,0 +1,189 @@ | |||
| // CLI utility to build/list/extract/run ESZIPs | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've vendoring the eszip module (so we don't do any remote imports at runtime), but somehow this file was missing. It's needed for the extraction logic, so I'm adding it here.
| // not exposing. | ||
| regexp: Record<string, RegExp> | ||
| // Exposing internal property that the `URLPattern` polyfill class is using. | ||
| type ExtendedURLPattern = URLPattern & { regexp: Record<string, RegExp> } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a weird one. I had to update target in the TypeScript config from es2019 to es2022 in order to use the cause property of Error. But once I did that, I was no longer seeing this regexp property being populated.
I think there's a difference in behaviour between the two versions of that property that make the child class emit a regexp property as undefined that overrides the one from the parent.
To work around that I stopped extending the class and just defining a custom type. I think this is cleaner anyway.
| // entirely, and what we're asserting here is that we emit a system log when | ||
| // import assertions are detected on successful builds. Also, running it on | ||
| // earlier versions won't work either, since those won't even show a warning. | ||
| test.skipIf(lt(denoVersion, '1.46.3') || gte(denoVersion, '2.0.0'))( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this runs for (and needs to pass on) all versions.
| func, | ||
| importMap, | ||
| deno, | ||
| functionPath, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just changed the signature to accept only the function path — which is all we need — instead of the full function. This makes it easier to then pass the file from the extracted ESZIP.
|
This pull request adds or modifies JavaScript ( |
We've been unable to update Deno to 2.x on our build system because there are edge functions out there relying on import assert directives, which have been deprecated and fully removed in Deno 2, which means any sites using that feature would start seeing failing builds.
While we want to eventually stop supporting this, we need a longer timeline to communicate to customers that their code needs to be updated, and we can't really wait for that to happen before we update to Deno 2.
So this PR gives us a stop-gap solution to keep supporting import assert in Deno 2.