Skip to content

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

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

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

yaacovCR
Copy link
Contributor

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 yaacovCR requested a review from a team as a code owner June 12, 2025 23:24
@yaacovCR
Copy link
Contributor Author

image

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

@yaacovCR yaacovCR changed the title switch to esm-only with dev/prod conditions controlling instanceOf switch to esm-only with development condition enabling dev-instanceOf-check Jun 12, 2025
@@ -12,6 +12,27 @@ import { Callout } from 'nextra/components'

# Breaking changes

## `graphql-js` === ESM-only secondary to improved `require(esm)` support
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 yaacovCR force-pushed the esm-with-instanceof branch from 7ed7d05 to 458bdf8 Compare June 19, 2025 03:09
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

Successfully merging this pull request may close these issues.

3 participants