"one instance of graphql" error when using esm package.

[Pokute]

When using esm package (https://github.com/standard-things/esm) even most simple package.json configurations (only dependancies are esm and graphql-yoga) results in the Error: Cannot use GraphQLSchema "[object Object]" from another module or realm. error when using imports.

node -r esm testWithImport.js will fail while node testWithRequire.js and node -r esm testWithRequire.js will work.

[IvanGoncharov]

@Pokute Can you please setup sample repo to reproduce this issue?

[IvanGoncharov]

@Pokute Can you also run npm ls graphql and post the output here?

[Pokute]

Repo for reproducing: https://github.com/Pokute/graphql-esm-bug

[Pokute]

@jdalton Do you have opinions on this?

[jdalton]

This happens if you were to add a new npm script called

"mjsImport": "node --experimental-modules serverWithImport.mjs"

and a test file serverWithImport.mjs:

import graphglYoga from 'graphql-yoga';
import { GraphQLObjectType, GraphQLSchema, GraphQLString } from "graphql";

const { GraphQLServer } = graphglYoga

var Something = new GraphQLObjectType({
    fields: {
        else: { type: GraphQLString },
    },
    name: 'Something',
})

var schema = new GraphQLSchema({
    query: new GraphQLObjectType({
        fields: {
            something: {
                type: Something,
            },
        },
        name: 'Query',
    }),
});

var server = new GraphQLServer({ schema });
const instance = server.start(); // defaults to port 4000

too.

The issue is that graphql-yoga is CJS (and loads the CJS entry of graphql) but you're loading the ESM version of graphql.

Because graphgl is running two entry points they create 2 separate instances (a limitation of their current dual approach). The way around it is for graphql to instantiate their instances in CJS and export them for the ESM entry to consume.

[IvanGoncharov]

@jdalton Thanks for detail response. I have a few questions about the proposed solution:

The way around it is for graphql to instantiate their instances in CJS and export them for the ESM entry to consume.

Is it something like this?
index.mjs

const {
  GraphQLObjectType,
  // ....
} = require('./graphql');

export GraphQLObjectType;
// ...

If so, it makes tree shaking impossible, right?
This library is pretty big and it's released as a single package so tree shaking is the only option to make it usable on the frontend.
So we can't make any changes that will break tree shaking support in Webpack or any other bundler.

[jdalton]

@IvanGoncharov

Is it something like this?

No. That wouldn't work in .mjs since require doesn't exist. The idea is that the GraphQLObjectType is defined in CJS and then used by the ESM build as well.

import GraphQLObjectType from "./types/object.js"

The other option is to drop instanceof checks and move to a check for the existence of say a non-enumerable symbol property.

[IvanGoncharov]

@jdalton Thanks for example.

The idea is that the GraphQLObjectType is defined in CJS and then used by the ESM build as well.

We can't split every type into a separate file since it will create a bunch of circular dependencies and also affect code readability.

The other option is to drop instanceof checks and move to a check for the existence of say a non-enumerable symbol property.

AFAIK, IE doesn't support Symbol so we can't use it.

---

IMHO, Having two instances of the same library is a problem by itself even if it doesn't trigger any errors. So even if we drop instanceof it doesn't solve the problem of having two instances of graphql it will just hide symptoms.

See Lee's comment here: #996 (comment)

In my opinion this is more of a bandaid than a real solution since the real problem is that this package evolves in capabilities over time and the types and classes exported by one version may not behave correctly with functions from another version. In addition there are deep assumptions made in the package about singletons like build in scalar types or introspection types which we check via identity. Even if we were to change instanceof to brand-checks like this, we're still stuck with multiple versions resulting in weird or broken behavior.

Moreover I would argue that this problem is not a something specific to this particular library since a lot of other libraries also use classes, singletons, etc.
So if ESM declares it supports MJS format it should ensure that only one instance of the library is created.

@Pokute For now, I think the most practical solution would be to open a feature request against graphql-yoga to also ship mjs files.

[jdalton]

@IvanGoncharov

AFAIK, IE doesn't support Symbol so we can't use it.

core-js has a shim for it. You could also use any-other property to detect the instance object.__COULD_BE_THIS_KIND_OF_PROPERTY__.

So if ESM declares it supports MJS format it should ensure that only one instance of the library is created.

This is not an esm issue as mentioned this presents itself with --experimental-modules too. The issue is that graphql has 2 entry points for the package.

For now, I think the most practical solution would be to open a feature request against graphql-yoga to also ship mjs files.

FWIW I'm a Node core member and a member of the Node Module Working Group. Our recommendation is to not adopt --experimental-modules for production use.

Update:

As of esm v3.0.83 we will resolve .js before .mjs to avoid this issue.

[arackaf]

Just removing the .mjs entry point might be a good interim solution. Is anyone even using .mjs in production? Plenty of people are using esm, and this is causing hard-to-track bugs.

[dimitrikochnev]

@jdalton thank you for v3.0.83!