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.

Sign up for GitHub

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

Jump to bottom

fix: vendor deno dependencies #2302

Merged

Skn0tt merged 14 commits into main from vendor-deno-deps

Sep 29, 2023

Contributor

Skn0tt commented Sep 25, 2023

Description

This PR vendors all dependencies of the edge function templates into the src/templates/vendor directory, and sets the functions[*].deno_import_map directive to use it at build-time. This should shield next-runtime builds against deno.land outages.

Documentation

Tests

I didn't add any tests - hoping our existing E2E suite will capture any regressions.

Relevant links (GitHub issues, etc.) or a picture of cute animal

Closes https://github.com/netlify/pod-ecosystem-frameworks/issues/404.


          fix: vendor deno dependencies

1addb01

Skn0tt requested a review from a team as a code owner

September 25, 2023 09:37

Skn0tt self-assigned this

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/6516b0b9f4604c0008729973
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-static-root-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/6516b0b9b04d9d0007fdac60
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/6516b0b9552a5c0008d299f9
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-export-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions bot added the type: bug label

Skn0tt commented

View reviewed changes

packages/runtime/package.json Outdated Show resolved Hide resolved

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/6516b0b98db0570008119aae
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-next-auth-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/6516b0b9ab0e000008ffc1dd
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for netlify-plugin-nextjs-demo-all-flags ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/netlify-plugin-nextjs-demo-all-flags/deploys/6516b0b91f93510008a5fa51
😎 Deploy Preview	https://deploy-preview-2302--netlify-plugin-nextjs-demo-all-flags.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for next-plugin-edge-middleware ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/6516b0b9ab0e000008ffc1d8
😎 Deploy Preview	https://deploy-preview-2302--next-plugin-edge-middleware.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/6516b0b9f64a05000926044d
😎 Deploy Preview	https://deploy-preview-2302--nextjs-plugin-custom-routes-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for next-i18next-demo ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/next-i18next-demo/deploys/6516b0b93eb4d80008465e2d
😎 Deploy Preview	https://deploy-preview-2302--next-i18next-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify bot commented Sep 25, 2023 •

edited

Loading

✅ Deploy Preview for next-plugin-canary ready!

Name	Link
🔨 Latest commit	`35886e1`
🔍 Latest deploy log	https://app.netlify.com/sites/next-plugin-canary/deploys/6516b0b91a4c0c000774611e
😎 Deploy Preview	https://deploy-preview-2302--next-plugin-canary.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Contributor Author

Skn0tt commented Sep 25, 2023

This is an eszip file that I created using the changes in this PR. Opening it on https://eszip-viewer.deno.dev/, you can see that it indeed picks up the vendored files from the local directory:

Screenshot 2023-09-25 at 11 40 17


          chore: install deno along node

4988b83

Contributor Author

Skn0tt commented Sep 25, 2023

For some reason, this import map breaks the config detection. I'll need to figure out what's going on there before we can merge this. Turning the PR into draft for now.

Edge Functions bundling                                       
────────────────────────────────────────────────────────────────

Packaging Edge Functions from .netlify/edge-functions directory:
 - next_middleware
 - next_pages_api_edge
Could not load configuration for edge function at '/Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/next_pages_api_edge/index.ts'
error: Module not found "file:///Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/vendor/deno.land/[email protected]/flags/mod.ts".
    at https://64e8753eae24930008fac6d9--edge.netlify.com/bootstrap/server.ts:1:23
Could not load configuration for edge function at '/Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/next_middleware/index.ts'
error: Module not found "file:///Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/vendor/deno.land/[email protected]/flags/mod.ts".
    at https://64e8753eae24930008fac6d9--edge.netlify.com/bootstrap/server.ts:1:23

Skn0tt marked this pull request as draft

September 25, 2023 10:53


          fix: install deno in cypress tests

88ac9a9

MarcL reviewed

View reviewed changes

packages/runtime/src/helpers/edge.ts Outdated

@@ @@ -378,6 +378,11 @@ export const writeEdgeFunctions = async ({ @@
                 const nextConfig = nextConfigFile.config
                 const usesAppDir = nextConfig.experimental?.appDir
+                await copy(getEdgeTemplatePath('../vendor'), join(edgeFunctionRoot, 'vendor'))
+                netlifyConfig.functions['*'] = {
+                  deno_import_map: join(INTERNAL_EDGE_FUNCTIONS_SRC, 'vendor', 'import_map.json'),

Contributor

MarcL Sep 25, 2023

This gives a TypeScript error as deno_import_map doesn't exist in FunctionsObject. Do we need to update the type to support import maps?

Contributor Author

Skn0tt Sep 25, 2023

yup, good catch! doing that in netlify/build#5305

Contributor

MarcL Sep 25, 2023

Let me know when it's released and when this PR is updated. We'll get this merged. 👍🏻

Skn0tt mentioned this pull request

fix: document deno_import_map in function types netlify/build#5305

Merged

5 tasks

Contributor

pieh commented Sep 26, 2023

For some reason, this import map breaks the config detection. I'll need to figure out what's going on there before we can merge this. Turning the PR into draft for now.

Edge Functions bundling                                       
────────────────────────────────────────────────────────────────

Packaging Edge Functions from .netlify/edge-functions directory:
 - next_middleware
 - next_pages_api_edge
Could not load configuration for edge function at '/Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/next_pages_api_edge/index.ts'
error: Module not found "file:///Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/vendor/deno.land/[email protected]/flags/mod.ts".
    at https://64e8753eae24930008fac6d9--edge.netlify.com/bootstrap/server.ts:1:23
Could not load configuration for edge function at '/Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/next_middleware/index.ts'
error: Module not found "file:///Users/skn0tt/dev/netlify/next-runtime/demos/middleware/.netlify/edge-functions/vendor/deno.land/[email protected]/flags/mod.ts".
    at https://64e8753eae24930008fac6d9--edge.netlify.com/bootstrap/server.ts:1:23

This seems like deno.land dep of netlify.edge? ( https://github.com/netlify/edge-functions-bootstrap/blob/721c691484c132849bca4b71339017ff93302e3a/src/bootstrap/server.ts#L1 ) and because we don't vendor that module here, but we point entire deno.land to stuff we vendored we just miss that module? Would it be possible to also add netlify edge to list of modules to be vendored? alternatively massage import map to target only modules that runtime uses and not entire deno.land?

Skn0tt added 6 commits

September 29, 2023 10:15


          fix: try scoping down the import map

30416b5


          fix: import straight from vendor directory

187aae1


          fix: run vendoring before testing deno

b5ae8db


          fix: npm install needs deno

4ffa6b1


          fix: run build:vendor in right dir

7201a27


          refactor: move deno tests into normal test file

Contributor Author

Skn0tt commented Sep 29, 2023

Would it be possible to also add netlify edge to list of modules to be vendored?

No, because it's a moving target and when we update the version here:

https://github.com/netlify/edge-bundler/blob/d5d427ab74a98d1bcd49e7d84fe227426f74f57d/deno/config.ts#L2

we might also update the stdlib modules under the hood, and then it'll break again since Next-runtime has other modules bundled.

alternatively massage import map to target only modules that runtime uses and not entire deno.land?

We could implement this using a scoped import map. I've tried that out with 30416b5, but wasn't able to get it working.

I'm now going down a straight-forward path, and am updating all imports to straight-up import from the vendor directory. Not the most beautiful, but workable until we figure out what's breaking the scoped import maps.

Skn0tt added 2 commits

September 29, 2023 11:23


          fix: include vendor files in package

1ec8485


          fix: patch html rewriter to fetch wasm from web, not from disk

e8428c8

Skn0tt commented

View reviewed changes

packages/runtime/html_rewriter.patch

@@ @@ -0,0 +1,4 @@ @@
+c56
+              <       fetch(new URL("./vendor/html_rewriter_bg.wasm", import.meta.url).href)
+              ---

Contributor Author

Skn0tt Sep 29, 2023 •

edited

Loading

This looks like we're replacing the from-disk fetch with a HTTP request at runtime here. This is required because deno vendor doesn't download the WASM onto disk (for whatever reason), so fetching this from disk would fail. This was already the case before this PR, but before this file was imported via https://, meaning that import.meta.url was something like https://deno.land/x/... - now that we're accessing this file from disk, it's more like file://.netlify/edge-functions/vendor.... Before, with the HTTPS address, fetch would fall back to an HTTP request anyways.

So TL;DR: This looks like we're replacing a solid disk-read with a brittle network fetch. In reality, it's always been a brittle network fetch, so this is fine-ish!

Contributor

pieh Sep 29, 2023

Can we download that wasm file ourselves and manually vendor it? i.e. manually pull from https://github.com/worker-tools/html-rewriter/blob/v0.1.0-pre.17/vendor/html_rewriter_bg.wasm and avoid doing a patch (it's bit icky to me, but manual download is icky as well, so I guess - pick your poison). This way at least we would avoid this brittle network fetch?

Contributor Author

Skn0tt Sep 29, 2023

Tried that, but the problem is not only that deno vendor doesn't download WASM files - Deno's eszip tool (that we use for bundling) also isn't able to detect the import, so it doesn't bundle the file. This will probably change once https://github.com/tc39/proposal-source-phase-imports lands, but until then the HTTP fetch is the only thing we can do :/


          Merge branch 'main' into vendor-deno-deps

226f5c7

Skn0tt marked this pull request as ready for review

September 29, 2023 10:32


          chore: use precise path

9a2b90d

Skn0tt requested a review from MarcL

September 29, 2023 10:35

pieh reviewed

View reviewed changes

packages/runtime/src/templates/vendor.ts Show resolved Hide resolved


          Update packages/runtime/src/templates/vendor.ts

35886e1

Co-authored-by: Michal Piechowiak <[email protected]>

pieh approved these changes

View reviewed changes

Contributor

pieh left a comment

Skn0tt merged commit a8d06ab into main

Skn0tt deleted the vendor-deno-deps branch

September 29, 2023 14:42

token-generator-app bot mentioned this pull request

chore: release main #2280

Merged

Skn0tt mentioned this pull request

fix: respect import map files containing only scopes netlify/edge-bundler#495

Merged

orinokai mentioned this pull request

[bug]: netlify dev fails to run middleware #2339

Closed

Skn0tt mentioned this pull request

fix: copy vendor over for local dev #2341

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels