withastro / adapters

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

Cloudflare adapter build fail when using pg lib #22

Closed jadbox closed 1 year ago

jadbox commented 1 year ago

Astro Info

Astro                    v3.2.3
Node                     v18.17.1
System                   Linux (x64)
Package Manager          bun
Output                   server
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

linux

Describe the Bug

Trying to run astro build on a Cloudflare project using the lib pg. PG is supported by cloudflare workers.

✘ [ERROR] Could not resolve "events"

    node_modules/pg/lib/native/client.js:12:27:
      12 │ var EventEmitter = require('events').EventEmitter
         ╵                            ~~~~~~~~

  The package "events" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "util"

    node_modules/pg/lib/native/client.js:13:19:
      13 │ var util = require('util')
         ╵                    ~~~~~~

  The package "util" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "net"

    node_modules/pg/lib/connection.js:3:18:
      3 │ var net = require('net')
        ╵                   ~~~~~

  The package "net" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "assert"

    node_modules/pg-protocol/dist/parser.js:9:41:
      9 │ const assert_1 = __importDefault(require("assert"));
        ╵                                          ~~~~~~~~

  The package "assert" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "net"

    node_modules/pg/lib/stream.js:6:22:
      6 │   const net = require('net')
        ╵                       ~~~~~

  The package "net" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "tls"

    node_modules/pg/lib/stream.js:21:20:
      21 │   var tls = require('tls')
         ╵                     ~~~~~

What's the expected result?

I'm not sure, but PG lib is supported by cloudflare. https://developers.cloudflare.com/workers/tutorials/postgres/#4-connect-to-the-postgresql-database-in-the-worker

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ig3cma?file=astro.config.mjs

Participation

EDIT by @alexanderniebuhr: more context at https://discord.com/channels/830184174198718474/1159635481902972928

jadbox commented 1 year ago

It seems like we may need the cloudflare adapter to expose the esbuild parameters. There's a related conversation here with svelte's cloudflare adapter: https://github.com/sveltejs/kit/pull/10521

jadbox commented 1 year ago

This thread also looks highly relevant: https://github.com/sveltejs/kit/issues/10028

alexanderniebuhr commented 1 year ago

@jadbox thank you for the report, I'll read through the linked issues, to understand why this happens. IMO the pg should not be supported by CF without the node: prefix.

We already support the nodejs_compact mode from Cloudflare, but only with the node: prefix, since this is the way it is documented.

jadbox commented 1 year ago

@alexanderniebuhr thanks for taking a look. I still do believe this is an Astro build bug around esbuild. Outside of Astro, Wrangler can use and deploy projects with PG today. (https://developers.cloudflare.com/workers/tutorials/postgres/)

alexanderniebuhr commented 1 year ago

Astro build bug around esbuild

It is the expected behaviour. We do support nodejs_compact, as it is described in the docs (https://developers.cloudflare.com/workers/runtime-apis/nodejs). If Cloudflare is starting to support Node.js API imports without the prefix node:*, I would like to see the docs for it first.

If we start to allow Node.js imports, without having the knowledge that it is supported, we will potential allow code, which we should indeed not build.

ematipico commented 1 year ago

@alexanderniebuhr thanks for taking a look. I still do believe this is an Astro build bug around esbuild. Outside of Astro, Wrangler can use and deploy projects with PG today. (developers.cloudflare.com/workers/tutorials/postgres)

Have you tried to put pg in the external configuration, so it won't be built?

jadbox commented 1 year ago

@alexanderniebuhr thanks for taking a look. I still do believe this is an Astro build bug around esbuild. Outside of Astro, Wrangler can use and deploy projects with PG today. (developers.cloudflare.com/workers/tutorials/postgres)

Have you tried to put pg in the external configuration, so it won't be built?

I have, and sadly it still trips up build issues for all the many sub libraries used by pg. Maybe it would work if I added add of these libs too, but it looks like overkill at this point.

I also now get an Astro Cloudflare adapter error for missing cloudflare:sockets directive to ignore it during build time.

jadbox commented 1 year ago

I've submitted a PR to get past the astro build error for the missing cloudflare:sockets lib within the Astro Cloudflare adapter. https://github.com/withastro/astro/pull/8766

I'm blocked from continuing to debug this issue until this is merged and deployed.

alexanderniebuhr commented 1 year ago

The remaining issues are due to a difference between node_compat & nodejs_compact in Cloudflare. While the first is used in some Cloudflare Tutorials it seems to be the older and unmaintained one. We do fully support the later one. Additionally to this we'll add support for custom esbuild settings in the future (#56), which will be tracked in a different issue.

That's the reason I'm going to close this issue.

ianpetzer commented 2 months ago

Hi,

I've run into a similar issue and I've found that cloudflare is compatible with postgres.js as opposed to pg. You can even see in the link from OP https://developers.cloudflare.com/workers/tutorials/postgres/#4-connect-to-the-postgresql-database-in-the-worker that the import is from postgres, not pg import postgres from "postgres";

I also needed to add the following to my astro config to get build to run: export default defineConfig({ output: 'server', adapter: cloudflare(), vite: { ssr: { external: ['node:events', 'node:buffer', 'cloudflare:sockets', 'node:stream'], }, build: { rollupOptions: { external: ['cloudflare:sockets'], }, }, }, });