kriasoft / react-starter-kit

The web's most popular Jamstack front-end template (boilerplate) for building web applications with React
https://reactstarter.com
MIT License
22.66k stars 4.16k forks source link

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

Closed langpavel closed 3 years ago

langpavel commented 6 years ago

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 commented 6 years ago

Can I propose a rewrite of the example schema with graphql-tools a la https://github.com/kriasoft/react-starter-kit/issues/1379#issuecomment-324509512? 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 commented 6 years ago

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

tim-soft commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

langpavel commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@tim-soft Hahaa, like same position :slightly_smiling_face: There are things which I think are the best, but trends far, far away are different :slightly_smiling_face: Like blindly breaking all into smaller bricks. Which makes sense sometimes, but I see so many really bad examples :slightly_frowning_face: 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 :money_mouth_face: more :moneybag:)

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

My lobby :smile: Thanks for patience. Criticism welcomed :+1:

frenzzy commented 6 years ago

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 commented 6 years ago

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:

I think I developed some interesting ideas which can save time :money_with_wings: 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. :100:

Hey sorry, I have a :beers: 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 commented 6 years ago

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 commented 6 years ago

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

tim-soft commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@langpavel How would you split with care?

langpavel commented 6 years ago

@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. :smiling_imp:

ulani commented 3 years ago

@langpavel thank you very much for crating this issue! Unfortunately, we have close it due to inactivity. Feel free to re-open it or join our Discord channel for discussion.

NOTE: The main branch has been updated with React Starter Kit v2, using JAM-style architecture.