ESM support

[PabloSzx]

Description

ESM Support

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Test Environment:

• OS: Windows & macOS
• @graphql-codegen/...: latest
• NodeJS: v14.17.0 & v16.3.0

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[changeset-bot]

🦋 Changeset detected

Latest commit: 07d3a5b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 53 packages

Name	Type
@graphql-codegen/typescript-resolvers	Minor
@graphql-codegen/typed-document-node	Minor
@graphql-cli/codegen	Minor
@graphql-codegen/cli	Minor
@graphql-codegen/core	Minor
@graphql-codegen/c-sharp	Minor
@graphql-codegen/c-sharp-operations	Minor
@graphql-codegen/flow	Minor
@graphql-codegen/flow-operations	Minor
@graphql-codegen/flow-resolvers	Minor
@graphql-codegen/java-apollo-android	Minor
@graphql-codegen/java-common	Minor
@graphql-codegen/java	Minor
@graphql-codegen/kotlin	Minor
@graphql-codegen/java-resolvers	Minor
@graphql-codegen/add	Minor
@graphql-codegen/fragment-matcher	Minor
@graphql-codegen/introspection	Minor
@graphql-codegen/jsdoc	Minor
@graphql-codegen/schema-ast	Minor
@graphql-codegen/time	Minor
@graphql-codegen/urql-introspection	Minor
@graphql-codegen/visitor-plugin-common	Minor
@graphql-codegen/typescript-apollo-angular	Minor
@graphql-codegen/typescript-apollo-client-helpers	Minor
@graphql-codegen/typescript-document-nodes	Minor
@graphql-codegen/typescript-enum-array	Minor
@graphql-codegen/typescript-generic-sdk	Minor
@graphql-codegen/gql-tag-operations	Minor
@graphql-codegen/typescript-graphql-files-modules	Minor
@graphql-codegen/typescript-graphql-request	Minor
@graphql-codegen/typescript-mongodb	Minor
@graphql-codegen/named-operations-object	Minor
@graphql-codegen/typescript-oclif	Minor
@graphql-codegen/typescript-operations	Minor
@graphql-codegen/typescript-react-apollo	Minor
@graphql-codegen/typescript-react-offix	Minor
@graphql-codegen/typescript-react-query	Minor
@graphql-codegen/typescript-rtk-query	Minor
@graphql-codegen/typescript-stencil-apollo	Minor
@graphql-codegen/typescript-type-graphql	Minor
@graphql-codegen/typescript	Minor
@graphql-codegen/typescript-urql	Minor
@graphql-codegen/typescript-urql-graphcache	Minor
@graphql-codegen/urql-svelte-operations-store	Minor
@graphql-codegen/typescript-vue-apollo	Minor
@graphql-codegen/typescript-vue-apollo-smart-ops	Minor
@graphql-codegen/typescript-vue-urql	Minor
@graphql-codegen/gql-tag-operations-preset	Minor
@graphql-codegen/graphql-modules-preset	Minor
@graphql-codegen/import-types-preset	Minor
@graphql-codegen/near-operation-file-preset	Minor
@graphql-codegen/plugin-helpers	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[PabloSzx]

I found an issue in CLI, and it's the usage of "require.resolve", which is not supported in ESM https://github.com/dotansimha/graphql-code-generator/blob/master/packages/graphql-codegen-cli/src/config.ts#L183

there isn't a good isomorphic solution to this issue, for more info check this: https://stackoverflow.com/questions/54977743/do-require-resolve-for-es-modules and https://nodejs.org/api/esm.html#esm_import_meta_resolve_specifier_parent

[PabloSzx]

Another issue found in @graphql-codegen/plugin-helpers https://github.com/dotansimha/graphql-code-generator/blob/master/packages/utils/plugins-helpers/src/resolve-external-module-and-fn.ts#L7

That will most definitely break in ESM

[dotansimha]

I found an issue in CLI, and it's the usage of "require.resolve", which is not supported in ESM https://github.com/dotansimha/graphql-code-generator/blob/master/packages/graphql-codegen-cli/src/config.ts#L183

Yeah, seems to be an issue. And we do use this in codegen core to load plugins, configurations, and loaders. I guess we need a wrapper for that to make sure it's still supported in both setups.

Another issue found in @graphql-codegen/plugin-helpers https://github.com/dotansimha/graphql-code-generator/blob/master/packages/utils/plugins-helpers/src/resolve-external-module-and-fn.ts#L7

That will most definitely break in ESM

How do you think we should address this one?

cc @ardatan

[PabloSzx]

How do you think we should address this one?

If that question is for me (maybe it wasn't), I feel like that is completely out of my scope since I'm not completely familiar with all the codegen internals, but adding support for esm I'd say is pretty much required for programmatic usage if for example GraphQL.js starts adding support for ESM with the exports field, that will trigger duplicated graphql module versions error, since if a commonjs module requires graphql, it will use the cjs version, and an esm module will use the esm version (this applies for every GraphQL library btw)

[dotansimha]

How do you think we should address this one?

If that question is for me (maybe it wasn't), I feel like that is completely out of my scope since I'm not completely familiar with all the codegen internals, but adding support for esm I'd say is pretty much required for programmatic usage if for example GraphQL.js starts adding support for ESM with the exports field, that will trigger duplicated graphql module versions error, since if a commonjs module requires graphql, it will use the cjs version, and an esm module will use the esm version (this applies for every GraphQL library btw)

Yeah, the question was for you :D
I see what you mean, so I think maybe we can wait a bit longer on this PR, and plan this for the next major version.

[PabloSzx]

@dotansimha @ardatan

While working in a example using graphql-codegen programmatically I realized that the plugin-helpers .mjs file is bundling requires that come from bundling change-case-all, in ESM world bundling dependencies most of the time is forbidden, because it will bundle the requires in the esm files, which of course breaks.

Of course the solution is to add "change-case-all" as a dependency in this specific package, but this applies to every package that intends to support esm

[PabloSzx]

After adding the test-esm.mjs script (and locally fixing the pending to be published fix of graphql-tools ardatan/graphql-tools#3289) these errors appear, this script is only able to catch errors in the imports, not the logic inside of the scripts

I4qPI3OXjD.mp4

I will take a look into fixing these issues

[PabloSzx]

After ardatan/graphql-tools#3300 is merged and publish, this PR should be ready @dotansimha

[PabloSzx]

createRequireFromPath is already removed in Node.js v16 (it was already deprecated in earlier versions), I will remove it and only use "createRequire", it should be fine since Node.js v10 support is already dropped

[ardatan]

Do we really need a major release?

[PabloSzx]

the changesets can be changed, I think this change is for minor, it was set as major since at first there was some changes that I thought were breaking, now everything should be fine

[PabloSzx]

@ardatan done 👍

[ardatan]

We don't publish testing and website to the NPM so those would changeset fail at NPM release. Could you remove these two?

[PabloSzx]

@ardatan done 👍 this issue in changesets has been open since so much time... changesets/changesets#436