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.

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

[Bug] SchemaLink and Async schema duplicated GraphQL instance #217

Open
luchillo17 opened this issue Feb 26, 2024 · 5 comments
Open

[Bug] SchemaLink and Async schema duplicated GraphQL instance #217

luchillo17 opened this issue Feb 26, 2024 · 5 comments

Comments

@luchillo17
Copy link

luchillo17 commented Feb 26, 2024

Based on #188 having a Schema that is created async will give us lots of problems trying to use SchemaLink, the current state is that we can avoid using an absolute URL for the HttpLink if we use SchemaLink on the SSR side, however, if the Schema is generated async we need to handle the promise before it even lands in the ApolloNextAppProvider's makeClient factory, which doesn't accept async factories.

The following was recommended as a way to handle the promise properly, it works and the site renders properly, but the SSR side gives an error about multiple graphql instances and the render takes longer than using HttpLink with absolute URL:

As for async client components, you might want to use use instead, but it will require a split into two components and a suspense boundary to prevent your clientFactory from being called all over again:

function ParentWrapper({children}){
  const [promise] = useState(clientFactory)
  return <Suspense><ChildWrapper promise={promise}>{children}</ChildWrapper></Suspense>
}
function ChildWrapper({promise, children}){
  const makeClient = use(promise)
  return <ApolloNextAppProvider makeClient={makeClient}>{children}</ApolloNextAppProvider>
}

We can't do that on a library level since it would wrap all our user's applications into an additional suspense boundary and that changes general UX, so it the developer needs to be very aware of that.

Tried the use and useState thing, it worked but suddenly got this kind of issue, I scanned the lock file, it should only exist a single version of graphql in my app, maybe it is creating multiple instances?

image

Originally posted by @luchillo17 in #188 (comment)

@luchillo17
Copy link
Author

luchillo17 commented Feb 26, 2024

Reproduction:

  • Clone https://github.com/luchillo17/graph-meister/tree/feature/schema-link-wrapper
  • Install dependencies
  • As an Nx monorepo, there are no npm scripts, have to run with nx run graph-budget:dev if installed globally, if not prepend with package manager run version like pnpm exec nx run graph-budget:dev.
  • Open the local URL that NextJS will show as output, usually http://localhost:4200.
  • Page will render eventually, but it takes longer than when using the HttpLink with absolute URL instead of SchemaLink, and an error will show in the terminal.

My thoughts:
First, I did try multiple stuff, checked the lock file to make sure only 1 version of graphql was installed, and even tried overrides to force all libraries that depend on graphql to use the same version regardless of the dependency listed (pnpm.overrides which is the equivalent of npm.resolutions prop), same issue regardless, so no, it is not an issue with the dependency version.

I think it might have to do with the way I'm using the async import to get my neo4j-driver only in the server, were I to convert this into a normal import it would break the server completely (couldn't even start it) because some of the code relies on NodeJS specific APIs, maybe the fact that my ApolloSSRWrapper gets run once per request might have something to do, or maybe my schema null check is not working as I expected to share the schema between all instances of the wrapper...
image

@phryneas
Copy link
Member

I'm gonna guess that you're running into a dual package hazard with the graphql module here.

I imagine your code is transpiled before being executed, so everything is a require, except for that one stray import.

The only thing I can imagine right now to get around that dynamic import would be to wrap that code into a new package that exposes different contents for different environments, based on a conditional exports field.

You can see a simple example of that here in rehackt - but you'd need to adjust it a bit:

{
  "exports": {
    ".": {
      "types": "./index.d.ts",
      "browser": "./empty.js"
      "default": "./actualCode.js"
    },
    "./package.json": "./package.json"
  },
}

That way, the browser build would always pick this up as an empty file.

I'm really sorry for all of these complications, but that's the sorry state the complexity of the React bundling experience puts us in right now.

@luchillo17
Copy link
Author

At this point given the requirement of the schema being generated async and this issue, RSC and Server Actions start to look more appealing, otherwise, I would have to stick to HttpLink, then again saving the first request for SSR might not be worth the effort, especially if the API and the UI are served from the same host.

@phryneas
Copy link
Member

I'd love to have a better solution for you here - you are pretty much experiencing every pain the ecosystem has to offer :(

That said, are you 100% sure that you need to dynamically import neo4j-driver? I imagine you could move that one or two files away and it would be tree-shaken completely in the browser if you sprinkle enough typeof window === "undefined" in there 🤔

@luchillo17
Copy link
Author

Well, the typings for the driver don't have an option that doesn't return a Promise (I might have to check the src and see if there's an internal sync option), meanwhile, I am wondering if trying to activate top-level await and maybe making the output target of TS be ES2022 or NodeNext or some other config like that if it could simplify my code and make it easier to export the Schema instead of wrapping in async functions as I do now...

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

No branches or pull requests

2 participants