"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.

[danturu]

@jdalton thank you for v3.0.83!

[IvanGoncharov]

@jdalton Thanks for fixing it in ESM.

[jdalton]

Just a heads up this doesn't fix graphql with Node's native --experimental-modules. Your problem still exists. Other projects like Create React App 2.0 are removing .mjs until issues, like these, can be worked out.

Update:

Related #1536.

[jaydenseric]

Just removing the .mjs entry point might be a good interim solution. Is anyone even using .mjs in production?

I am, and several popular published packages depend on it.

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

That's bad news, people rely on the propper Node.js resolution order.

This issue is being over complicated. graphql-yoga should provide a .mjs build then the issue will go away. As a bonus native ESM in Node.js would be supported without having to use the esm package.

[jdalton]

@jaydenseric

That's bad news, people rely on the proper Node.js resolution order.

It's not so bad when the alternative is non-working code. For esm the resolution order is configurable via options.cjs.paths (defaults to true).

This issue is being over complicated. graphql-yoga should provide a .mjs build then the issue will go away. As a bonus native ESM in Node.js would be supported without having to use the esm package.

👋 Node core and Node Module WG member here. There is no set date for when --experimental-modules will be unflagged. There's also no guarantee that what has shipped now is what will be in the future. For example, there are several discussions in the Node Module WG on how to approach implementation. One such discussion is to ship a maximally minimal implementation that would require package-maps for resolving bare specifiers (no longer using the Node module resolution algorithm).

Adopting .mjs with the idea that its current support is solid (for production use) at this stage is bit premature and may cause headaches for users. I've already seen .mjs cause issues for real projects like create-react-app, pm2, react-apollo, and webpack due to complications with transpilers/bundlers, testing/mocks, or dual packages. Also, if/when changes do happen to Node's --experimental-modules your ESM story may be various shades of broken.

The --experimental-modules flag really does mean experimental. It's something you should pause and consider at least before going all in as the route for your ESM support strategy today.

[hos]

My solution was to replace

import graphql from 'graphql'

with

import graphql from 'graphql/index.js'

I was using flags --experimental-modules --es-module-specifier-resolution=node to omit extensions.