Skip to content
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

Bundling error when used with NextJS #1826

Open
tim-field opened this issue Oct 22, 2023 · 4 comments · May be fixed by #1985
Open

Bundling error when used with NextJS #1826

tim-field opened this issue Oct 22, 2023 · 4 comments · May be fixed by #1985
Labels
🐛 bug

Comments

@tim-field
Copy link
Contributor

tim-field commented Oct 22, 2023 •
edited

Summary

Unable to import postgraphile (v5) or grafast in an next js project.

Attempting to setup a route that includes postgraphile v5 but next refuses to bundle

 ⚠ ./node_modules/graphile-build-pg/dist/pgServices.js
Critical dependency: the request of a dependency is an expression        

Import trace for requested module:
./node_modules/graphile-build-pg/dist/pgServices.js
./node_modules/graphile-build-pg/dist/index.js
./node_modules/postgraphile/dist/index.js
./src/lib/graphile/pgl.ts
./src/app/graphql/route.ts

//pgl.ts
import preset from './graphile.config'
import postgraphile from 'postgraphile'

export const pgl = postgraphile(preset)

Steps to reproduce

Create a new next js app using npx create-next-app@latest

install postgrahile@beta I was using ^5.0.0-beta.15

Attempt to setup a route which imports postgraphile

Expected results

Next js able to complile / bundle

Actual results

A bundling error

Additional context

@benjie thinks this is a bundling error

It’s a bundling issue; I’m afraid I’ve not put time into supporting that yet. Please file an issue with the above details; it sounds like I’ll need to change the configuration format to avoid a dynamic require

https://discord.com/channels/489127045289476126/1165551144299401317/1165574782675329056

image

Possible Solution
@benjie
Copy link
Member

benjie commented Oct 23, 2023

This comes down to loading a named adaptor:

crystal/graphile-build/graphile-build-pg/src/pgServices.ts

Lines 29 to 49 in 7ff3d50

function reallyLoadAdaptor<
TAdaptor extends
keyof GraphileConfig.PgDatabaseAdaptorOptions = keyof GraphileConfig.PgDatabaseAdaptorOptions,
>(adaptorString: TAdaptor): PromiseOrDirect<PgAdaptor<TAdaptor>> {
try {
const adaptor = require(adaptorString);
return adaptor?.createWithPgClient ? adaptor : adaptor?.default;
} catch (e) {
if (e.code === "ERR_REQUIRE_ESM") {
const importSpecifier = adaptorString.match(/^([a-z]:|\.\/|\/)/i)
? pathToFileURL(adaptorString).href
: adaptorString;
const adaptorPromise = import(importSpecifier);
return adaptorPromise.then((adaptor) =>
adaptor?.createWithPgClient ? adaptor : adaptor?.default,
);
} else {
throw e;
}
}
}

Ideally we'd pass the adaptor directly, but I have a feeling we're doing TypeScript stuff involving the adaptor name so it might be a bit of effort. That's what we should do though.

If anyone fancies tearing out this code and replacing it with passing the adaptor directly via the configuration (so the user explicitly imports it themself) and also updating the documentation in the various places it's referenced, that would be appreciated 👍

hannesj added a commit to hannesj/crystal that referenced this issue Mar 7, 2024
@hannesj
Make adapter in config the concrete adapter instance, not import path
3d9199f 
Fixes graphile#1826
@hannesj hannesj linked a pull request Mar 7, 2024 that will close this issue
Open
5 tasks
hannesj added a commit to hannesj/crystal that referenced this issue Mar 27, 2024
@hannesj
Make adapter in config the concrete adapter instance, not import path
c16e519 
Fixes graphile#1826
hannesj added a commit to hannesj/crystal that referenced this issue Apr 23, 2024
@hannesj
Make adapter in config the concrete adapter instance, not import path
6dfe5ef 
Fixes graphile#1826
@benjie
Copy link
Member

benjie commented May 10, 2024

Relates to #1985

@psirenny
Copy link

@tim-field did you find a workaround to this issue?

@psirenny
Copy link

We're working around the issue by marking dependencies as external:

/** @type {import('next').NextConfig} */
const nextConfig = {
  experimental: {
    esmExternals: true,
    serverComponentsExternalPackages: [
      "grafast",
      "grafserv",
      "graphile-export",
      "graphql",
      "postgraphile",
      "ruru",
    ],
  },
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐛 bug
Projects
V5.0.0
Status: 🌱 In Progress
Milestone
No milestone
3 participants
@benjie @psirenny @tim-field