refactor: simplify AST converter, generate `.json` configs instead `.ts` #822

dimaMachina · 2021-12-05T19:40:39Z

UPDATE: I removed all unrelated changes to this PR to make review simple

refactor: generate .json configs instead .ts
fix some broken links in docs
use valueFromASTUntyped from graphql-js
don't export functions from ./estree-parser, but export requireGraphQLSchemaFromContext and requireSiblingsOperations

changeset-bot · 2021-12-05T19:40:41Z

⚠️ No Changeset found

Latest commit: 875e67c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

theguild-bot · 2021-12-05T19:41:09Z

The latest changes of this PR are not available as alpha, since there are no linked changesets for this PR.

dimaMachina · 2021-12-05T19:43:27Z

packages/plugin/src/estree-parser/estree-ast.ts

  ? SafeGraphQLType<T> &
      Pick<BaseNode, 'leadingComments' | 'loc' | 'range'> & {
        type: T['kind'];
-        gqlLocation: Location;


this is typo, must be Pick<Location, 'start' | 'end'>

Update: I removed this field because is unused in graphql-eslint, also this info can be retrieved with range field

dimaMachina · 2021-12-17T12:17:11Z

docs/custom-rules.md

 export const rule = {
  create(context) {
-    requireGraphQLSchemaFromContext(context)
+    requireGraphQLSchemaFromContext('your-rule-name', context)


dimaMachina · 2021-12-17T12:17:29Z

docs/custom-rules.md

        const typeInfo = node.typeInfo()
-
-        if (typeInfo && typeInfo.gqlType) {
+        if (typeInfo.gqlType) {


typeInfo is always truthy

dimaMachina · 2021-12-17T12:18:26Z

docs/parser-options.md

+With this configuration, you can specify custom configurations for GraphQL's `parse` method. By default, `graphql-eslint` parser just adds `noLocation: false` to make sure all parsed AST has `location` set, since we need this for tokenizing and for converting the GraphQL AST into ESTree.

-You can find the [complete set of options for this object here](https://github.com/graphql/graphql-js/blob/master/src/language/parser.d.ts#L7)
+You can find the [complete set of options for this object here](https://github.com/graphql/graphql-js/blob/6e48d16f92b9a6df8638b1486354c6be2537033b/src/language/parser.ts#L73)


fixed broken link with permanent link

dimaMachina · 2021-12-17T12:18:38Z

docs/parser-options.md

 ```

-> The configuration here is flexible, and will be sent to `graphql-tools` and it's loaders. So depends on the schema source, the options may vary. [You can read more about these loaders and their configuration here](https://www.graphql-tools.com/docs/api/interfaces/_loaders_graphql_file_src_index_.graphqlfileloaderoptions).
+> The configuration here is flexible, and will be sent to `graphql-tools` and it's loaders. So depends on the schema source, the options may vary. [You can read more about these loaders and their configuration here](https://graphql-tools.com/docs/api/interfaces/loaders_graphql_file_src.GraphQLFileLoaderOptions#properties).


fixed broken link

dimaMachina · 2021-12-17T12:20:02Z

packages/plugin/tests/__snapshots__/parser.spec.ts.snap

        Object {
          description: Object {
            block: true,
-            gqlLocation: Object {


I don't see a reason to have gqlLocation field, currently is unused, also this info can be retrieved with range field

dimaMachina · 2021-12-17T12:26:31Z

scripts/generate-configs.ts

+    rules: getRulesConfig('Schema', true),
+  });
+
+  writeFormattedFile('configs/operations-recommended.json', {


the main purpose of this is to have configs in separate places instead of having them in index.js file.
So this will be easy to look up for debugging purposes.

dimaMachina · 2021-12-17T12:30:48Z

README.md


 - 🚀 Integrates with ESLint core (as a ESTree parser)
- 🚀 Works on `.graphql` files, `gql` usages and `/* GraphQL */` magic comments
+- 🚀 Works on `.graphql` files, `gql` usages and `/* GraphQL */` magic comments


replaced with non-breaking space, so on mobile, this must look better

before

after

dimaMachina · 2021-12-17T12:44:22Z

packages/plugin/src/estree-parser/utils.ts

-  }, Object.create(null));
-}
-
-export function valueFromNode(valueNode: GraphQLESTreeNode<ValueNode>, variables?: Record<string, any>): any {


use valueFromASTUntyped instead of copy this fn

refactor: generate .json configs instead typescript fix links use `valueFromASTUntyped` from graphql-js don't export functions from `./estree-parser`, but export `requireGraphQLSchemaFromContext` and `requireSiblingsOperations`

dimaMachina commented Dec 5, 2021

View reviewed changes

dimaMachina marked this pull request as ready for review December 5, 2021 19:48

dimaMachina marked this pull request as draft December 5, 2021 21:31

dimaMachina changed the title ~~refactor: simplify AST converter~~ refactor: simplify AST converter, generate .json configs instead .ts Dec 6, 2021

dimaMachina force-pushed the refactor-ast-converter branch from 35885f4 to 07b34cb Compare December 17, 2021 12:15

dimaMachina commented Dec 17, 2021

View reviewed changes

dimaMachina marked this pull request as ready for review December 17, 2021 12:22

dimaMachina commented Dec 17, 2021

View reviewed changes

dimaMachina force-pushed the refactor-ast-converter branch from 07b34cb to 7e3c4dd Compare January 2, 2022 15:28

refactor: simplify AST converter

875e67c

refactor: generate .json configs instead typescript fix links use `valueFromASTUntyped` from graphql-js don't export functions from `./estree-parser`, but export `requireGraphQLSchemaFromContext` and `requireSiblingsOperations`

dimaMachina force-pushed the refactor-ast-converter branch from 7e3c4dd to 875e67c Compare February 1, 2022 14:21

dimaMachina merged commit b86425f into master Feb 16, 2022

dimaMachina deleted the refactor-ast-converter branch February 16, 2022 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: simplify AST converter, generate `.json` configs instead `.ts` #822

refactor: simplify AST converter, generate `.json` configs instead `.ts` #822

Uh oh!

dimaMachina commented Dec 5, 2021 •

edited

Loading

Uh oh!

changeset-bot bot commented Dec 5, 2021 •

edited

Loading

Uh oh!

theguild-bot commented Dec 5, 2021 •

edited

Loading

Uh oh!

dimaMachina Dec 5, 2021

Uh oh!

dimaMachina Dec 17, 2021 •

edited

Loading

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021 •

edited

Loading

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

dimaMachina Dec 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

refactor: simplify AST converter, generate .json configs instead .ts #822

refactor: simplify AST converter, generate .json configs instead .ts #822

Uh oh!

Conversation

dimaMachina commented Dec 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Dec 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

theguild-bot commented Dec 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaMachina Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaMachina Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

before

after

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

refactor: simplify AST converter, generate `.json` configs instead `.ts` #822

refactor: simplify AST converter, generate `.json` configs instead `.ts` #822

dimaMachina commented Dec 5, 2021 •

edited

Loading

changeset-bot bot commented Dec 5, 2021 •

edited

Loading

theguild-bot commented Dec 5, 2021 •

edited

Loading

dimaMachina Dec 17, 2021 •

edited

Loading

dimaMachina Dec 17, 2021 •

edited

Loading