Adding experimental online parser by 0xnmn · Pull Request #2770 · graphql/graphql-js

0xnmn · 2020-08-29T22:05:30Z

This PR includes implementation of an experimental online parser based on JSON rules set. This parser can be used with IDEs to implement syntax highlighting and auto suggestions. It is a state-full parser which parser the source string incrementally i.e. emits the next token each time.

IvanGoncharov · 2020-08-30T10:09:03Z

@@ -0,0 +1,2 @@
+export { Parser } from './parser';
+export * from './types';


Can you please rename the folder to experimentalOnlineParser?

IvanGoncharov · 2020-08-30T10:16:01Z

+export interface LanguageType {
+  rules: Rules;
+}
+export interface Rules {


Can you add GraphQLGrammar prefix to types for JSON ruleset?

IvanGoncharov · 2020-08-30T10:18:32Z

+export declare type Styles = {
+  [name: string]: string;
+};
+export declare type ParserConfig = {


For types related to parser can you please use OnlineParser prefix?

IvanGoncharov · 2020-08-30T10:21:18Z

+export declare type ParserConfig = {
+  tabSize: number;
+};
+export declare type ParserConfigOption = {


Can you please move Parser specific types into onlineParser.js?
and rename it to grammarTypes?

IvanGoncharov · 2020-08-30T10:21:49Z

@@ -0,0 +1,2 @@
+export { Parser } from './parser';
+export * from './types';


Please use explicit exports and expose only parser related types.

IvanGoncharov · 2020-08-30T10:25:21Z

@@ -0,0 +1 @@
+export { default } from './rules';


so just remove this file

IvanGoncharov · 2020-08-30T10:31:53Z

+export class Parser {
+  state: ParserState;
+  lexer: Lexer;
+  styles: Styles;


Can you please move it to GraphiQL code?

IvanGoncharov · 2020-08-30T10:32:56Z

+  state: ParserState;
+  lexer: Lexer;
+  styles: Styles;
+  config: ParserConfig;


Suggested change

config: ParserConfig;

_config: ParserConfig;

IvanGoncharov · 2020-08-30T10:34:35Z

+
+export declare class Parser {
+  state: ParserState;
+  lexer: Lexer;


Suggested change

lexer: Lexer;

_lexer: Lexer;

IvanGoncharov · 2020-08-30T10:34:44Z

+} from './types';
+
+export declare class Parser {
+  state: ParserState;


Suggested change

state: ParserState;

_state: ParserState;

IvanGoncharov · 2020-08-30T10:36:37Z

+  sol(): boolean;
+  parseToken(): Token;
+  indentation(): number;
+  private readonly parseTokenConstraint;


Can you please rename them and add just add underscore?

IvanGoncharov · 2020-08-30T10:37:16Z

+  private readonly getNextRule;
+  private readonly popMatchedRule;
+  private readonly rollbackRule;
+  pushRule(


public methods should be grouped together.

IvanGoncharov · 2020-08-30T10:39:36Z

+    state: ?ParserState,
+    styles: ?Styles,
+    config: ?ParserConfigOption,
+    source: string,


Nono-ptional parameters should go first.

IvanGoncharov · 2020-08-30T10:41:36Z

+    styles,
+    config,
+    source,
+  }: {|


Can you please convert to positional arguments?
especially since we need to remove styles.

IvanGoncharov · 2020-08-30T14:53:28Z

+  [name: string]: GraphQLGrammarRule;
+}
+
+export declare type GraphQLGrammarRule =


Please remove declare.

IvanGoncharov · 2020-08-30T14:54:23Z

@@ -0,0 +1,76 @@
+// @flow


Please remove these lines in all files

IvanGoncharov · 2020-08-30T14:56:39Z

@@ -0,0 +1,938 @@
+{
+  "rules": {


Please remove rules wrapper.

We might want to integrate this with ast later on or many include lex rules. So that is why I nested it.

@hereisnaman In this case rules and lexerRules would be confusing so we would need to rename it anyway.
It's a not a stable API so we can always introduce wrapper latter if needed.

IvanGoncharov · 2020-09-06T08:52:26Z

@@ -0,0 +1,936 @@
+{


It looks like Deno doesn't support importing JSON:
denoland/deno#5633

Can you please convert it to rules.js and also merge it with grammarTypes.js?
On the plus side, we would be able to type-check rules content with TS.
Also please name the resulting file grammar.js.

If it triggers a coverage check please add necessary files to list of excluded files inside .nycrc.

I'll update.

0xnmn force-pushed the advanced-parser branch 2 times, most recently from a51166b to 608c3a7 Compare August 29, 2020 22:19

IvanGoncharov added the PR: feature 🚀 requires increase of "minor" version number label Aug 29, 2020

Adding experimental online parser

375de5b

0xnmn force-pushed the advanced-parser branch from 608c3a7 to 375de5b Compare August 29, 2020 22:24

IvanGoncharov reviewed Aug 30, 2020

View reviewed changes

Improve Typings

eba0620

IvanGoncharov reviewed Aug 30, 2020

View reviewed changes

0xnmn mentioned this pull request Aug 30, 2020

[WIP][reference] Integrate graphql-js experimental parser graphql/graphiql#1653

Closed

IvanGoncharov mentioned this pull request Sep 2, 2020

Launch GraphQL Foundation Monthly Newsletter graphql/foundation#26

Open

Update grammar structure

e2a3881

IvanGoncharov reviewed Sep 6, 2020

View reviewed changes

Move rules to js

3acae6a

IvanGoncharov force-pushed the advanced-parser branch from eba0adb to 4bfd46e Compare September 13, 2020 11:28

Fix build

6b10dbe

IvanGoncharov force-pushed the advanced-parser branch from 4bfd46e to 6b10dbe Compare September 17, 2020 14:15

IvanGoncharov merged commit 13ece49 into graphql:master Sep 17, 2020

yaacovCR mentioned this pull request Apr 25, 2026

Plans to reintroduce online parser? #4691

Open

		@@ -0,0 +1,2 @@
		export { Parser } from './parser';
		export * from './types';

Conversation

0xnmn commented Aug 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IvanGoncharov Aug 30, 2020 • 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!

IvanGoncharov Aug 30, 2020 • 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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IvanGoncharov Aug 30, 2020 •

edited

Loading

IvanGoncharov Aug 30, 2020 •

edited

Loading