Urigo / merge-graphql-schemas

A utility library to facilitate merging of modularized GraphQL schemas and resolver objects.
https://www.npmjs.com/package/merge-graphql-schemas
929 stars 66 forks source link

fileLoader crashes when loading js file #98

Closed benjhoo closed 5 years ago

benjhoo commented 7 years ago

Hey, still me and the fileLoader() function :-)

I have an issue when I use this function to load js file in my Meteor project.

Here one of my graphql types in JS :

export default `
  type Author {
  id: ID!
  firstName: String
  lastName: String
}

type Query {
  authors: [Author]
  author(id: ID!): Author
}
`;

And in my main file which merge the types :

const types = fileLoader(_myTypesFolder_);
export default mergeTypes(types);

And Meteor returns me the following error :

(function (exports, require, module, __filename, __dirname) { export default `
                                                              ^^^^^^
SyntaxError: Unexpected reserved word

However, if I use .graphql for my types files instead of .js, everything work fine. If I manually import the types files to merge them in the main file, it works too.

I think it's a ES6 problem with export and the fileLoader function.

RichardLitt commented 7 years ago

Thanks for logging this, @benjhoo!

@cfnelson I think this should properly be labelled as a support question, as I don't think it's a bug and I think it's probably a local issue. What do you think about adding a label for that?

cfnelson commented 7 years ago

@benjhoo Do you have a custom .babelrc setup in your Meteor project?

@RichardLitt Thanks for bringing this to my attention. I agree, it should be support question! 👍

benjhoo commented 7 years ago

@cfnelson No, I don't use any custom .babelrc

cfnelson commented 7 years ago

@benjhoo Sorry for the late response. Are you still experiencing this issue? If so are you on the latest release of this package? I suspect that there is something occurring with the meteor setup.

If your able to create a minimal reproduction of this problem it would go a long way to help us resolve the issue.

benjhoo commented 7 years ago

Hi @cfnelson My turn to be sorry for the late response, I was on holidays.

Yes, unfortunately, I still have this issue. I have the latest release of the package : 1.3.0 Maybe it's a compatibility problem with Meteor packages. merge-graphql-schemas is looking for graphql@0.10.3, while 0.11.7 version is installed by Meteor.

You can find a small reproduction of my project in my Git repo : https://github.com/benjhoo/merge-graphql-meteor.

Thank you.

cfnelson commented 7 years ago

@benjhoo I had a quick look and I am unsure of the exact cause. However I thought I should point out that when you run meteor build the bundle will not contain these .js files.

Personally I have used this package in the past with Meteor by importing/exporting .graphql files. You can add .graphql support in Meteor with this package https://github.com/Swydo/meteor-graphql

I will try to take a deeper look when I get the chance and will ping @RodMachado to see if he has any ideas.

benjhoo commented 7 years ago

@cfnelson thank you for your response.

Yes, it works when I use .graphql extension instead of .js for the schemas (As I mentioned in my first post). No need to use an additional package apparantly. But the problem remains when I try to autoload the resolvers .js files.

I will wait for a possible correction of this problem so. I will manually import the .js files in the meanwhile.

Thank you.

vuldin commented 6 years ago

I'm running into this same issue, and it was worked around as @cfnelson said by loading the resolver files manually.

Here is a resolver that doesn't work with a folder of resolver files with js extensions:

import path from 'path'
import { fileLoader, mergeResolvers } from 'merge-graphql-schemas'

const dir = path.join('src', 'resolvers')
const resolversArray = fileLoader(dir, { extensions: ['.js'] })

export default mergeResolvers(resolversArray, { all: true })

Here is a working resolver file that manually loads the needed files (as index.js within the resolver folder):

import { mergeResolvers } from 'merge-graphql-schemas'
import post from './post'
import todo from './todo'

const resolvers = [post, todo]

export default mergeResolvers(resolvers)

The error I get when trying to use the non-working version above is SyntaxError: Unexpected token import, which is similar to the error given in the initial comment. This makes me think that it's related to babel not being used to transpile the files before es5-only code needs to make sense of them.

cfnelson commented 6 years ago

@vuldin are you using webpack? If so could we see your config so we can try to reproduce the exact behaviour your experiencing.

vuldin commented 6 years ago

I uploaded the project I was working on last night here. Links to a few relevant files:

Sorry about the convoluted resolver file changes, all in interest of keeping the names the way I want them :( . Let me know if you are able to reproduce easily, otherwise I can make some changes so that there aren't any complications around the being able to test out the non-working automated resolver.

vuldin commented 6 years ago

I went ahead and separated the resolvers into three versions: one that is following the manual approach and works, and two others (one and two) that fail at attempting to load the resolvers with the fileloader.

Each attempt gives a different error, and I've included the error for each file in a comment at the top of the associated file. In order to switch between the files, just uncomment the appropriate line around this spot.

vuldin commented 6 years ago

It looks like the issue is with the dynamic require here. I don't know of any easy way to fix this with rollup, but it looks like webpack supports dynamic import in recent versions. See here for how the syntax works.

I changed my resolvers to cjs modules (along with any dependent modules) and it works fine. The changes I needed to make (plus a couple of unrelated ones) are here: https://github.com/vuldin/apollo-mongo-webpack/commit/8f898944b3b4674e1053eeff0f8d84fc0471eb34

cfnelson commented 6 years ago

@vuldin Thanks heaps for the detailed response & investigation! We suspected the require statement you pointed out was the cause as well. Will spend some time taking a look today on the best way to fix/resolve this issue.

joserocha3 commented 5 years ago

@cfnelson, any progress on this. I messed around with the current version with no luck.

RichardLitt commented 5 years ago

Thanks for pinging, @joserocha3. We'll most likely have time for this after the holiday season. Hope that is OK. :)

ardatan commented 5 years ago

It looks related to how to load modules; because you need to register a loader in NodeJS that supports ESM syntax such as babel-node or esm.