aerogear / graphback

Graphback - Out of the box GraphQL server and client
https://graphback.dev
Apache License 2.0
409 stars 73 forks source link

Cannot find module 'graphql' #1530

Closed craicoverflow closed 4 years ago

craicoverflow commented 4 years ago

Version: 0.14.0-alpha6

The CLI is not working in alpha6 because graphql is missing.

❯ npx graphback-cli@0.14.0-alpha6 init fullstack-issue           
npx: installed 521 in 28.088s
Cannot find module 'graphql'
Require stack:
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/node_modules/@graphback/core/dist/crud/mappingHelpers.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/node_modules/@graphback/core/dist/crud/index.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/node_modules/@graphback/core/dist/index.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/node_modules/graphback/dist/index.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/dist/components/generate.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/dist/components/index.js
- /home/ephelan/.npm/_npx/396521/lib/node_modules/graphback-cli/dist/index.js

This is because generate command uses the graphback package, which in turn looks for a peer dependency.

craicoverflow commented 4 years ago

Any update on what will happen with client document generation? Will this stay in the main graphback CLI? This is the only reason we still have the generate command.

wtrocki commented 4 years ago

This is documentation issue. Generate should not be called from global scope but be part of your project scripts (that have desired graphql versions)

However thereis valid question here that blocks us (in terms of the breaking changes etc.) to make call about generate command as whole.

I see the perfect scenario when:

craicoverflow commented 4 years ago

Generate should not be called from global scope

True if you already have a project. But this happened on graphback init, which would mean you do not have a project yet.

wtrocki commented 4 years ago

Graphback init calls generate? The reason we removed GraohQL was that init do not do generate and require GraphQL

craicoverflow commented 4 years ago

It does not call it, but it looks as though index.js loads it by by exporting the code:

https://github.com/aerogear/graphback/blob/61dad9f25f99cdc881a2510fbdb301a1c009ee99/packages/graphback-cli/src/index.ts#L4

I can take a look after, but there is possibly no need for this to be exported from here.

craicoverflow commented 4 years ago

I'm still seeing this issue on 0.14.0-alpha13.

The fix applied in #1541 was verified with by executing node $GRAPHBACK/packages/graphback-cli/dist/index.js init <project-name> but this must still acquire the peer dependency through $GRAPHBACK/node_modules/graphql.

I don't see working until either:

  1. init command is removed and you can only create a new project through graphql-cli or create-graphql. I see this coming after 0.14.0 is released as graphql cli will have a lot of breaking changes from this release.
  2. generate command is removed and moved to Offix (in progress or planned already). When is the target timeline to move client document generation to Offix @wtrocki @kingsleyzissou ?
craicoverflow commented 4 years ago

\cc @machi1990

kingsleyzissou commented 4 years ago

This isn't currently being developed just yet for Offix. It's on the roadmap, but there are other changes that need to land before we are able to approach this.

craicoverflow commented 4 years ago

Thanks. Option 1 seems much more favourable right now then.

wtrocki commented 4 years ago

CLi in offix will be lower priority. Also doing hard cliexit will remove capabilities from plugins. For example we do services and building custom resolvers without context types is very very hard. Like this would be nr one reason that people would bail from Graphback. If we have ability to drop plugin and generate some typing that will definitely make it better.

I do not want to be back to generating resolvers business, however I think we make certain failstart for this.

While init can go away to create-graphql (and probably makes sense to even rebrand the package as it is), but not sure about generate. Let's discuss this as this is technical problem and we mixing it with features/architecture problems

wtrocki commented 4 years ago

We agreed to resolve this by creating a separate package - create-graphback

wtrocki commented 4 years ago

see https://github.com/aerogear/graphback/issues/1624