ESM support

[PabloSzx]

Description

This is also a fix, since #486 was already merged and currently the project can't build without these changes

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: 20a741b

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

This PR includes changesets to release 1 package

Name	Type
@graphql-eslint/eslint-plugin	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 am thinking that doesn't make sense for graphql-eslint to support ESM if eslint itself is all commonjs, and there is a couple places that the translation for esm is not straight forward:

https://github.com/dotansimha/graphql-eslint/blob/master/packages/plugin/src/testkit.ts#L26

https://github.com/dotansimha/graphql-eslint/blob/master/packages/plugin/src/rules/graphql-js-validation.ts#L56 does eslint support async validation to add inline dynamic imports?

In that case the solution for the current build error is to rollback the bob-the-bundler version, and add to bob-the-bundler an option to skip all the esm validation and building

PS: #486 shouldn't have been merged, the CI didn't catch the building error because "yarn build" it's not in the github actions workflow

[dotansimha]

I am thinking that doesn't make sense for graphql-eslint to support ESM if eslint itself is all commonjs, and there is a couple places that the translation for esm is not straight forward:

https://github.com/dotansimha/graphql-eslint/blob/master/packages/plugin/src/testkit.ts#L26

https://github.com/dotansimha/graphql-eslint/blob/master/packages/plugin/src/rules/graphql-js-validation.ts#L56 does eslint support async validation to add inline dynamic imports?

In that case the solution for the current build error is to rollback the bob-the-bundler version, and add to bob-the-bundler an option to skip all the esm validation and building

PS: #486 shouldn't have been merged, the CI didn't catch the building error because "yarn build" it's not in the github actions workflow

Seems like ESLint does support ESM in some cases (like config file, see https://eslint.org/docs/user-guide/configuring/configuration-files#configuration-file-formats and eslint/eslint#14304 ) . Not sure about parser/plugin and the rest of the features we are using. Since they are being loaded eventually by ESLint core, and we should expose code that is compatible with that.

I agree that we can rollback Bob and then think of the next steps of this. What are you thoughts? @PabloSzx

[PabloSzx]

I agree that we can rollback Bob and then think of the next steps of this. What are you thoughts? @PabloSzx

Since bob-the-bundler version was already rolled-back the next steps are allowing bob-the-bundler to skip esm building, I did a quick read of eslint core and everything is plain js with commonjs, and is unlikely that eslint will do to change to esm in the short therm considering all the current ecosystem around it, so I would simply close this issue and add the bob-the-bundler configuration to commonjs only

[dotansimha]

I agree that we can rollback Bob and then think of the next steps of this. What are you thoughts? @PabloSzx

Since bob-the-bundler version was already rolled-back the next steps are allowing bob-the-bundler to skip esm building, I did a quick read of eslint core and everything is plain js with commonjs, and is unlikely that eslint will do to change to esm in the short therm considering all the current ecosystem around it, so I would simply close this issue and add the bob-the-bundler configuration to commonjs only

Sounds good, I agree.