withastro / adapters

Home for Astro's core maintained adapters
69 stars 41 forks source link

netlify adapter unable to handle node builtin imports #285

Closed theoephraim closed 4 months ago

theoephraim commented 5 months ago

Astro Info

Astro                    v4.10.2
Node                     v20.11.1
System                   macOS (arm64)
Package Manager          pnpm
Output                   server
Adapter                  @astrojs/netlify
Integrations             dmno-astro-integration

Describe the Bug

When using Netlify edge functions, it is possible to use node internal packages by importing them using the node: prefix. These imports do not work in their deno runtime otherwise.

The netlify astro adapter takes over bundling from netlify, and uses esbuild in platfiorm: neutral mode. In this mode, built-in node imports do not seem to supported, regardless of using the node: prefix or not. Here is an example, trying both a node:prefixed and unprefixed import.

image

note that the repro doesn't show the errors (or complete the build properly) when running on stackblitz, but if you clone the repo and run locally you'll see it.

What's the expected result?

At the very least, I would expect that node: prefixed imports would work as they do on netlify. Taking it one step further, we could alter the imports of node modules that are missing the prefix to add it - which can be useful if the import was created by a dependency and not by the user themselves.

This can be easily fixed with a simple esbuild plugin

 await build({
  // ...
  plugins: [{
    name: 'allowNodePrefixedImports',
    setup(build) {
      build.onResolve({ filter: /^node:.*$/ }, (args) => ({ path: args.path, external: true }));
    },
  }],
  // ...

and an example showing a plugin that adds of the node: prefix. Although there could be cases where this is not the intended behaviour.

const NODE_BUILTIN_MODULE_REGEX = /^(assert|buffer|child_process|cluster|crypto|dgram|dns|domain|events|fs|http|https|net|os|path|punycode|querystring|readline|stream|string_decoder|timers|tls|tty|url|util|v8|vm|zlib)$/;

// plugin to add missing `node:` prefix - which is required for deno
function addNodeImportPrefix() {
  return {
    name: 'addNodeImportPrefix',
    setup(build: any) {
      build.onResolve({ filter: NODE_BUILTIN_MODULE_REGEX }, (args: any) => ({
        path: `node:${args.path}`,
        external: true,
      }))
    },
  }
}

Link to Minimal Reproducible Example

https://stackblitz.com/~/github.com/theoephraim/astro-netlify-adapter-node-imports-bug

Participation

serhalp commented 4 months ago

Sounds like https://github.com/withastro/adapters/pull/286 fixed this? Good to close?