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

Cloudflare adapter build fail when using pg lib #22

Closed
1 task done
jadbox opened this issue Oct 6, 2023 · 10 comments
Closed
1 task done

Cloudflare adapter build fail when using pg lib #22

jadbox opened this issue Oct 6, 2023 · 10 comments

Comments

@jadbox
Copy link
Contributor

jadbox commented Oct 6, 2023

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

  • I am willing to submit a pull request for this issue.

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

@jadbox
Copy link
Contributor Author

jadbox commented Oct 6, 2023

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: sveltejs/kit#10521

@jadbox
Copy link
Contributor Author

jadbox commented Oct 6, 2023

This thread also looks highly relevant: sveltejs/kit#10028

@alexanderniebuhr
Copy link
Member

@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
Copy link
Contributor Author

jadbox commented Oct 6, 2023

@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
Copy link
Member

alexanderniebuhr commented Oct 6, 2023

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
Copy link
Member

@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
Copy link
Contributor Author

jadbox commented Oct 6, 2023

@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
Copy link
Contributor Author

jadbox commented Oct 6, 2023

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

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

@alexanderniebuhr
Copy link
Member

alexanderniebuhr commented Nov 7, 2023

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.

@alexanderniebuhr alexanderniebuhr closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2023
@ianpetzer
Copy link

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'],
},
},
},
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants