ardatan/graphql-import

Innocent comment causing error: Too few regex matches: null

Closed this issue · 5 comments

danielkcz commented

In our schema, there is a piece like this...

type ImportedOrder {
  # import errors
  error: String
}

With graphql-config lib I am reading such a schema and it fails with

(node:83264) UnhandledPromiseRejectionWarning: Error: Too few regex matches: null
    at parseImportLine (app\node_modules\graphql-import\src\index.ts:48:11)
    at Array.map (<anonymous>)
    at parseSDL (app\node_modules\graphql-import\src\index.ts:74:6)
    at collectDefinitions (app\node_modules\graphql-import\src\index.ts:237:22)
    at Object.importSchema (app\node_modules\graphql-import\src\index.ts:91:7)
    at Object.readSchema (app\node_modules\graphql-config\lib\utils.js:137:51)
    at GraphQLProjectConfig.getSchema (app\node_modules\graphql-config\lib\GraphQLProjectConfig.js:75:28)

The obvious culprit is here...

https://github.com/prisma/graphql-import/blob/b6890a0379b764c9adbf43e651988cbcf5105add/src/index.ts#L72

I am not entirely sure what's the purpose of that. Why not to actually use that regex instead of matching strings like that? 😆

https://github.com/prisma/graphql-import/blob/b6890a0379b764c9adbf43e651988cbcf5105add/src/index.ts#L46

Workaround

Had to remove that comment from schema before running the code. Luckily coworker was ok with changing it on server-side, so it won't bugger us for now, but it would be nice to fix it nonetheless.

ardatan commented

@FredyC I think it would be better to use ''' instead of # to prevent conflicts with import syntax. I also added a better error message in the beta version so you can try it out.

Also in 1.0.0 beta release, we introduced a lot of changes.
Could you install graphql-import@beta to try new changes? Don't forget to modify your code regarding to the migration notes in README.
https://github.com/ardatan/graphql-import#updating-from-07x

Let us know if your issue still persists.

danielkcz commented

@FredyC I think it would be better to use ''' instead of # to prevent conflicts with import syntax. I also added a better error message in the beta version so you can try it out.

Import syntax in schema SDL? Not sure what that would be good for :)

Either way, I no longer work on the project and since it was a one-time problem, a comment was changed on the backend side. No big deal.

May I suggest adding a test case for that instead of "crossed-fingers" it should work? :) Should be fairly simple imo.

ardatan commented

I think there is a misunderstanding :) I suggested you to use ''' instead of # for your comments to prevent conflicts with graphql-import's current import statements. I didn't mean any new method to define imports.
I see, what kind of tests do you mean?

danielkcz commented

@ardatan Well, the ''' is fairly ugly and verbose, to be honest. The library shouldn't be forced to use either of those because it cannot handle something correctly imo.

I see, what kind of tests do you mean?

I am not sure where is parsing happening in 1.x, but you have a test case in OP which should pass without an error.

ardatan commented

Available in 1.0.0!

  • 👍1