sangria-graphql / sangria-relay-playground

An example of GraphQL server supporting Relay written with Play and sangria
Other
17 stars 11 forks source link

Minor improvements #1

Closed OlegIlyenko closed 8 years ago

OlegIlyenko commented 8 years ago

Thank you very much for taking your time and making it!! You even updated the logo, it's just awesome! :trophy:

This PR may look pretty big (1474 files changed :D), but in reality it's not :) I just did something that I should have done in sangria-playground from the beginning: I removed all vendor assets (bootstrap, ace editor, jquery, etc) and bundle them with webpack :) I guess now it the good time to do it since webpack is already in place.

I also improved webpack bundling a bit. Now there is a separate bundle for every page (index and relay app) + I splited javascript and css. So after the webpack command, layout will like this:

I also made some minor changes. Would appreciate if you could review this PR. If you don't like some of the changes of think that it can be improved, just let me know - would be happy change this PR.

I reviewed the project and everything looks good to me :+1: I guess the next step would be to figure out how to automatically generate this pesky schema.json file with sbt.

yasserkaddour commented 8 years ago

Don't mention it, I wouldn't have done it without you and your awesome Sangria, I kinda preferred when webpack and npm was just for the minimal react-relay app, it was easier to get rid of the unnecessary asset for a starter-kit.

For the schema.json, there was onStart method of globalSettings but it's being deprecated, they use dependency injection instead, but I am not sure I am using it right. And I would prefer a solution regenerating the json on hot reload or only on schema change.

OlegIlyenko commented 8 years ago

I kinda preferred when webpack and npm was just for the minimal react-relay app, it was easier to get rid of the unnecessary asset for a starter-kit.

I agree. Bootstrap and all this jquery stuff obfuscates the rest a bit. What do you think about creating additional webpack.app-only.js or something like this which will bundle only the relay app? As fat as I saw, it's pretty common practice withing webpack community. As you mentioned, it would be helpful for people who would like to start new sangria-relay based project without playground part.

I think creating a standalone example app can be also an interesting option (just pain app, without playground part). WDYT?

Concerning the schema.json. I think it will require an sbt plugin, so that when you sbt ~run your application, it will generate the schema on every scala source change. It also would be nice to have an sbt task like sbt generateSchema which can be used outside of play.

Using onStart should be possible with the new DI as well. But one need to orchestrate the whole chain:

change in scala → sbt/play updates schema.json → webpack --watch caches this change and updates the bundles → app can be refreshed in browser (with webpack-dev-server --hot it hopefully will be refreshed automatically)

It would be interesting experiment to try and implement this chain. Dev productivity boost :)

yasserkaddour commented 8 years ago

A webpack.app-only.js with clear instruction seems enough.

I will try to implement the sbt-plugin and chain tomorrow.

Thanks.