The Wayback Machine - https://web.archive.org/web/20200710055806/https://github.com/kriasoft/react-starter-kit/issues/1443
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segregation of `src/data/queries` and `src/data/types` is contra-intuitive #1443

Open
langpavel opened this issue Oct 24, 2017 · 16 comments
Open

Comments

@langpavel
Copy link
Collaborator

@langpavel langpavel commented Oct 24, 2017

Too wide (and falsy) segregation of GraphQL Types and Resolvers

The first example of this is new ObjectType in src/data/queries/index.js importing fields.
Fields should be taken in context of parent type, not as standalone information.

This can ensure some users that custom types cannot have own field resolvers which is false.

In fact, much of fields which can have own resolvers are handled level up (which is possible but not always desired) and makes thinking about GraphQL harder than it actually are.

There is place where I need collect all the opinion and experience. Please, comments are welcomed!

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 26, 2017

Can I propose a rewrite of the example schema with graphql-tools a la #1379 (comment)? This opens up the possibility of schema stitching and I believe also solves your problem pretty easily?

I'm still of the opinion that writing the schema in straight JS feels a little contra-intuitive.

What is too wide is the authorization, could use a query/mutation example with some sort of access control

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 26, 2017

@tim-soft Believe that it should be done on master branch, agree?
Then I need prepare PR and get 👍 from @koistya and @frenzzy..

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 26, 2017

I think so. Going off my example, I'd probably introduce dataloader for the database calls -- it sets a good example for performance reasons, including a custom JSON type could also be helpful.

I'd be happy to throw a PR together for it, if even just for an example. While we're talking about it, it'd be pretty neat to be able to stitch a node-api-starter schema with RSK.

Also Apollo Client 2.0 plans?

@frenzzy
Copy link
Member

@frenzzy frenzzy commented Oct 26, 2017

Please read this thread: #1430
We are going to remove data layer from master branch of RSK and use cloud API as an example.
PRs with backend improvements are welcome to NSK.
New project ASK is a monorepo for RSK + NSK (frontend and backend in a single repository).

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 26, 2017

@frenzzy will the feature/apollo branch need to follow suit? Those are some very interesting changes to the architecture

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 27, 2017

I know that some changes are nice and unique, but it does not make sense to diverge it much..
In fact I was thinking about complete fork but I declined it because there is comunity on RSK an mine fork will be alone..
and I will need maintain it.. alone? — nonsense.
@tim-soft Can you lift up changes which cannot be reproduced, even on monorepo (ASK)?

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 27, 2017

@langpavel It's a tricky position because I only use the Apollo libs (Server, Subscriptions + Redis, Client) in production, so kinda divergent even from the Apollo branch. I could only support something like that. What would you wanna do?

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 27, 2017

@tim-soft Hahaa, like same position 🙂
There are things which I think are the best, but trends far, far away are different 🙂
Like blindly breaking all into smaller bricks. Which makes sense sometimes, but I see so many really bad examples 🙁
Yes, it make sense to break API from frontend if they are two things,
but in RSK this is no case, well only in Apollo branch.

Why?

Because if you break the two, you will spend some heat on unnecessary parsing/serialization/HTTP/parsing JSON and back (bleh) and all of this is eliminated by direct query. Yes, in fact, only for first request.

But in fact, most of requests are first. Bots, new visitors.. almost 80 percent of traffic is NEW

So, shortest pipeline is the best way how to tell arbitrary visitor arbitrary data in the shortest way. (See, less 🤑 more 💰)

Without wasting TCP and HTTP or socket.. just ask your DB directly.
In the almost perfect stack which can separate this all, IF NEEDED 😈

My lobby 😄 Thanks for patience.
Criticism welcomed 👍

@frenzzy
Copy link
Member

@frenzzy frenzzy commented Oct 27, 2017

The reasons why we don't want to have data layer in master branch are following:

  1. Many RSK users don't want to write their own backend. It is easier and probably cheeper to just use cloud api (like Firebase).
  2. Some people which chooses RSK already have backend, often maintained by someone else or written in php or other languages.
  3. It should be easy to use RSK for static websites with no backend. It's tiring to cut out the data layer every time you need to create one more simple website.
  4. People from dev teams usually uses microservices architecture in terms of responsibility where different people works on different parts of application, and it is ok for them to keep frontend and backend separately.
  5. Some guys misunderstanding how data layer works and they are tying to use backend only dependencies (for example send requests to db) inside routes (which executed in browser too) which makes me sad.

The ASK project serves as an example of how you can use RSK with your own backend where NSK is a possible option. I think there are nginx must be configured out of the box with reverse proxy, proper caching etc. I believe that microservices architecture is the best choose for such kind of projects. We can also add examples in the future for example how to add a service for health monitoring, image resources management etc.

In terms of performance I think each web app must respond with less than 50ms (TTFB) and this can not be achieved without caching, SSR is slow even without db requests. So, one internal http request for data between services will not affect performance significantly.

Meanwhile I think it is ok to have the Apollo branch in RSK (or in ASK) where all in one (frontend and data layer). Just the master branch is for the majority.

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 30, 2017

Ahoy @frenzzy!

I appreciate your work and you teach me a lot, but I discovered RSK as best kit at the time months ago which can do all the things in harmony.
Best for startups!
I see trends and obviously it makes sense, but I discovered shortcuts which can save a lot of CPU time in this kit.
And all is about configuration (may be compile time branching) and ops optimization (— not needed in KIT but in servers configuration later).
About development performance and understanding:

  • There is no developer performance without understanding
  • There is no understanding without documentation (most painful for every OSS project) and hehe support. 😈
  • There is no expected result without joining all the things which needs be together (Almost every big company do this in the really bad way)

I think I developed some interesting ideas which can save time 💸 and radically decrease SSR time. Yeah, I visited a lot of companies in past, where they break all into microservices. Yeah, they are in development phase since today or they abandoned the project. Why? Don't let managers drive technical backend. Never. Sorry. But they will create margins but no roads.

Shortest way how to speedup SSR is no cache on backend — how you can cache APIs? — No.
Shortest way is to cache entire response and you only need to know which one can be cached.
This is completely different question then RSK should solve — this should be solved by caching proxy like Varnish.

RSK should be the fastest — as the things together do the thing in most effective way. (Sorry, nobody knows about ASK, NSK,… as I know, no reference from README.md…)

You can broke RSK and deploy it thousand times, sometime as API backend only, sometimes as SSR only, sometimes as the one thing, just by server config. Domain proxy will decide. but the shortcut between GraphQL and SSR can be unreproducible and only thing which is likely violated is the trend — the technical separation of concerns which is health is kept by convention. 💯

Hey sorry, I have a 🍻 but just my spot.

On the point: Segregation of src/data/queries and src/data/types

Data resolvers are placed badly. Fields should have resolvers. Types have inlined resolvers somewhere, looking like chicken—egg.
Because every object type have fields, it looks strange having them in separate directories when you grow. It is not strange to create recurrent hiearchies, but you cannot reproduce this in directory layout. It's hard in mind too — but you can flatten this more by joining type and resolvers into one direstory instead to two files seaprately in one level up — like the types and queries is bad segregation, you should have type and specific field resolver more close – together.

Just do cp src/data/types/* src/data/ and cp src/data/queries/* src/data/ and you will be in better hiearchy than today

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 30, 2017

I see room for both, it's hard to check all the boxes in one boilerplate. Having it all in one app is very handy, but also having the ability to add on API endpoints(on separate servers) for high volume, computationally intensive tasks would also provide a lot of value in a generic boilerplatey sort of way.

I know what I'd like to do with the Apollo branch and possibly make an Apollo+PostgresQL based NSK that can be added via schema stitching, but I'm not sure others would have the same interest

¯_(ツ)_/¯

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 30, 2017

@tim-soft Having all together as a mess is not goal of anybody 😄
I personally use PostgreSQL and some internal LRU cache in very specific way.. in this combination it can reduce round-trip to microseconds 🎉. As an example.. (yeah, RSS from foreign source is not great example which makes sense 😄

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 30, 2017

Nobody likes a mess! I also have some strange use cases, like crawling/scraping which can fully consume a servers resources. You can't please everyone ;)

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 30, 2017

@tim-soft Ok, I write a long essay which nobody wants read probably. So in short, All–in–one can be more maintainable than many separate one–purpose things. This is more madness than you can achieve with reasonable cumulation — see how many projects are going to monorepo — because they solve some same-thing. Like RSK in beggining. One HTTP request from start to end. I think this is true ATM too.
Breaking to separate projects can be done with care. Because this is still RSK which I'm using because it can handle server, client, both. And in elegant way. This was the reason why I started using this in tie when I can choose from tenths project which wasn't fit to my All-in-one stuff :-) I'm single or we are working on in small group.. for startups.. that was RSK. Isomorphics as presented itself. Now it will not be isomorph. It will be splitted. Breaking promise

@tim-soft
Copy link

@tim-soft tim-soft commented Oct 30, 2017

@langpavel How would you split with care?

@langpavel
Copy link
Collaborator Author

@langpavel langpavel commented Oct 30, 2017

@tim-soft Good question.
Simplest answer is if you can compile server entirely without single bit of client code, for example. But this is only on you, if you wish keep it this way. 😈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.