switch to esm-only with development condition enabling dev-instanceOf-check

[yaacovCR]

Takes the no-op trick from #4426, but responding in a small way to feedback from @glasser in terms of discoverability, uses the development condition to enable this behavior by default.

The development condition must be passed to node via a flag node --conditions=development ... and is not set automatically from NODE_ENV=development.

[yaacovCR]

where we get the import-only warning, we are actually relying on esm-require

[glasser]

For what it's worth, I do think this will hinder adoption due to popular testing libraries like Jest still having pretty terrible support for using ESM-only modules even when Node and TypeScript are in better shape. While I think graphql-js should go here eventually, it will be disappointing if 3+ years worth of incremental improvements are made inaccessible to users who don't want to fight with their test tooling. I think it would be better to get out a GraphQL v17 that has the huge number of small improvements (but no major changes to the build system, implementation of dev mode, etc) and then rapidly release a GraphQL v18 that ONLY has these "build level" changes, so that users can separate their "application-level" migration from their "build system" migration.

[yaacovCR]

The underlying motivation is that require-esm supposedly “just works” in the latest versions of Node, so that our users using Jest would not be impacted.

I hope to push another alpha to allow the broader community to experiment with it.

[glasser]

I encourage you to try it out yourself. I tried making a simple package (@as-integrations/express4) ESM-only and found that it worked great when run directly from a Node CJS project but that trying to get it to work with Jest (while the project was still CJS-based) was hopeless.

[yaacovCR]

Hmmm, maybe I could work from a project you are getting errors with.

I tried to set up a greenfield project with jest + babel and/or ts-jest and everything was fine testing wise even prior to versions of node supporting require(esm) because the transpilation fixed everything. I am sure I am doing something wrong. Presumably people have other settings that limit transpilation of node_modules? although i did basically just follow the recommended defaults?

https://github.com/yaacovCR/require-esm-test

[yaacovCR]

@glasser similar demonstration in apollo-server for a canary taken from this PR:

apollographql/apollo-server#8082

These changes seem pretty straight-forward => once we are comfortable dropping support for the earlier versions of Node.

Some esm package with different setups might be trickier. For example, I looked randomly at @sindresorhus/is and that required bumping the module and moduleResolution to node16 from node, for reasons that I have yet figured out. Maybe it has to do with the types condition on exports in that package? Not sure. In any case, our package setup seems more flexible.

Would love to get additional feedback on when we do this. At first I wanted to push for v17, but with the points @glasser was making, I became fearful that despite require(esm) support, we might have to hold off even longer. Considering how basic these changes are for our library, I am wondering if we can go back to including in v17, and would push for that, but barring that, I am certainly comfortable with the suggestion above of releasing v17 without this change but prioritizing a v18 with this change.