sysgears / apollo-universal-starter-kit

Apollo Universal Starter Kit is a SEO-friendly, fully-configured, modular starter application that helps developers to streamline web, server, and mobile development with cutting-edge technologies and ultimate code reuse.
https://apollokit.org
MIT License
1.68k stars 323 forks source link

Add PersistGraphQL support during development and production #94

Closed larixer closed 7 years ago

larixer commented 7 years ago

This is a ticket to track progress of adding PersistGraphQL support during development and production.

The Pull Request by @mitjade to add PersistGraphQL support logic: https://github.com/sysgears/apollo-fullstack-starter-kit/pull/46

The effort to implement Webpack Plugin for PersistGraphQL: https://github.com/sysgears/persistgraphql-webpack-plugin The plugin works at the moment for:

The PersistGraphQL is based on more subtle plugin that implements concept of Webpack Overlay Modules - that live in memory additionally to modules from file system. These in-memory modules could be manipulated by higher order plugins to have means of communication between webpack compiler instances and source code through hot module replacement mechanisms: https://github.com/sysgears/webpack-overlay-modules Webpack Overlay Modules Plugin needs documentation, full-test coverage and submission to npm registry. This starter kit could benefit from Webpack Overlay Modules concept to have more robust implementation of front-end reloading on back-end changes

larixer commented 7 years ago

@mitjade I think PersistGraphQL plugin is ready for alpha testing: https://github.com/sysgears/persistgraphql-webpack-plugin

I have added code to optionally use PersistGraphQL plugin: https://github.com/sysgears/apollo-fullstack-starter-kit/commit/5a553fe76f58af7475535a99ed01e7a03b992280

Provided we have plugin for Webpack now it would be great to finish off the Pull Request in https://github.com/sysgears/apollo-fullstack-starter-kit/pull/46 and merge it via squash so that people can read how to add PersistGraphQL support in one commit.

What are your thoughts?

mitjade commented 7 years ago

@vlasenko Great! Will give it a try, and let you know.

mitjade commented 7 years ago

@vlasenko Tried https://github.com/sysgears/apollo-fullstack-starter-kit/commit/5a553fe76f58af7475535a99ed01e7a03b992280 and it looks fantastic! Will try to add what I have done in #46 and commit to your branch. Then I can make a new PR out of it.

larixer commented 7 years ago

@mitjade Cool. Sounds great! Thank you!

mitjade commented 7 years ago

@vlasenko I tried to add code from #46 to the origin/persistgraphql, but only now realised that frontend queryMap in src/client/main.js is empty. Backend one in src/server/webpage.js gives correct output?

larixer commented 7 years ago

@mitjade Oh, sorry. I've just pushed the fix to persistgraphql-webpack-plugin version 0.1.5

mitjade commented 7 years ago

@vlasenko I pushed my changes I have done in #46 to origin/persistgraphql branch. I am not able to make it work. Whenever I add the networkInterface = addPersistedQueries(networkInterface, queryMap); I get the following error: Network error: Could not find query inside query map.

But the code below in src/server/api_server.js never get's triggered:

  app.use(
    '/graphql',
    (req, resp, next) => {
      req.body = req.body.map(body => {
        return {
          query: invertedMap[ body.id ],
          ...body
        };
      });
      next();
    },
  );

If I comment out addPersistedQueries the code above gets triggered.

This seemed to work correctly before. Do you have any idea what I am doing wrong?

larixer commented 7 years ago

@mitjade The same error happens if instead of webpack plugin use json generated by persistgraphql CLI tool. I think we are facing this issue: https://github.com/apollographql/persistgraphql/issues/13

mitjade commented 7 years ago

@vlasenko So maybe this was working before I added post example, which uses fragments.

mitjade commented 7 years ago

@vlasenko For the next 2 weeks I will be busy with other work, so if you want to move this forward sooner, you can try to investigate your self, otherwise I will when I have more time.

larixer commented 7 years ago

@mitjade Yes, of course, thanks!

larixer commented 7 years ago

@mitjade I have looked at this issue. The main problem was that typename fields were absent in persisted queries and we are using network transport with typenames. I have added option to add typenames to generated queries to Webpack plugin and ironed out couple bugs. I believe now persisted queries work as expected

larixer commented 7 years ago

@mitjade I believe now https://github.com/sysgears/apollo-fullstack-starter-kit/pull/46 is ready for merge?

mitjade commented 7 years ago

@vlasenko A new PR probably needs to be made from origin/persistgraphql, or can this somehow be merged into #46? I can try it in the evening, but if it worked for you, please merge it.

larixer commented 7 years ago

@mitjade Mitja I have prepared origin/persistgraphql branch. Could you make me a favor and create PR from this branch? I want PR to be on your name.

mitjade commented 7 years ago

@vlasenko np, but you have done way more work on this matter than me, so it this is just about that no need :)

larixer commented 7 years ago

Fixed by: https://github.com/sysgears/apollo-fullstack-starter-kit/commit/dfd6e4e4ba16188718322b8baefb9cbbc33c6ca8