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

[Content]: Problems with Data Loading documentation #861

Open
amirhhashemi opened this issue Aug 28, 2024 · 9 comments · May be fixed by #991
Open

[Content]: Problems with Data Loading documentation #861

amirhhashemi opened this issue Aug 28, 2024 · 9 comments · May be fixed by #991
Assignees
Labels
pending review Awaiting review by team members.

Comments

@amirhhashemi
Copy link
Contributor

📚 Subject area/topic

Data loading in SolidStart

📋 Page(s) affected (or suggested, for new content)

https://docs.solidjs.com/solid-start/building-your-application/data-loading

📋 Description of content that is out-of-date or incorrect

The Data Loading documentation has several areas for improvement:

Structure: The organization is counterintuitive, with only two main sections: "Data loading on the client" and "Data loading always on the server" (oddly, a subsection of the former). This structure doesn't effectively guide users through the various data loading methods.
Comparison: There's a lack of clear comparison between different data loading methods (createAsync, createResource, and Tanstack Query). The documentation doesn't provide guidance on when to use each method.
Depth: Some crucial concepts, like the load function, are inadequately explained. The documentation doesn't clearly outline when and how to use this function effectively.
Examples: The documentation lacks comprehensive examples, particularly:

  • Using createResource with Suspense and ErrorBoundary
  • Implementing Tanstack Query
  • Demonstrating createAsync with actions

The proposed PR (#857) aims to address these issues.

🖥️ Reproduction in StackBlitz (if reporting incorrect content or code samples)

No response

@amirhhashemi amirhhashemi added improve documentation pending review Awaiting review by team members. labels Aug 28, 2024
@amirhhashemi amirhhashemi changed the title [Content]: [Content]: Problems with Data Loading documentation Aug 28, 2024
@atilafassina
Copy link
Member

Thanks, @amirhhashemi . You raise good points. But I think they need to be tackled separately, Let me break them down so we can start organizing on how to tackle them.

Structure: The orgaization is counterintuitive, with only two main sections: "Data loading on the client" and "Data loading always on the server" (oddly, a subsection of the former). This structure doesn't effectively guide users through the various data loading methods.

I agree, loading on the server should not be a subsection.
It also needs a preloading section and a mutation one (addressing form submission, pending UI, and single-flight mutations).

For that I'd propose we break it into 2 PRs:

  • Decoupling server / client data-loading. Putting preload into its own section.
  • Add a section for mutations and address pending and single-flight

Comparison: There's a lack of clear comparison between different data loading methods (createAsync, createResource, and Tanstack Query). The documentation doesn't provide guidance on when to use each method.

The difference between createAsync and createResource is pointed out in the reference page of each API. It warrants a callout in this section with links pointing to the difference.

  • Add remark to createAsync vs createResource to the Data Loading guide

I disagree that we should have a mention to Tanstack Query in the official guide of the framework for Data Loading. But I think it would be a great learning resource and perhaps we could even welcome it as a separate guide (within our docs, similar to what Migration from 0.9 is in the Solid-Router).

Depth: Some crucial concepts, like the load function, are inadequately explained. The documentation doesn't clearly outline when and how to use this function effectively.

I believe that would be addressed with adding a section to preload that I already added to your first comment, above in this message. Correct if I'm wrong though.


@LadyBluenotes if you and @amirhhashemi are ok with my suggestions, I'm glad moving labelling this as planned and adding the points above as subtasks of this same issue.

@amirhhashemi
Copy link
Contributor Author

amirhhashemi commented Dec 20, 2024

Thank you @atilafassina for taking a look at this.

First, I want to point out that I'm a beginner when it comes to Solid. So please correct me if I'm wrong about something. However, I have a lot of experience with React and its ecosystem.

I generally disagree with your suggestions. Here are my thoughts: (some of them in response to your suggestions and some of them more general)

  1. The title "data loading on client/server " is confusing. With createAsync the data loading code could run on both client and server. Maybe it's simpler to drop these terminologies and just point out that the fetcher function can be server only.
  2. I think we should differentiate createResource and createAsync more clearly. AFAIK, createAsync is a wrapper around createResource but it comes with its ecosystem and the ecosystem is not compatible with createResource.
  3. If this is a Getting Started guide and not a reference page, I think it should give enough information to the users so not only do they understand the different data loading methods, but they can decide which one they want to use in their app. They shouldn't have to go to two other pages just to take that basic decision. That is the responsibility of this page.
  4. I agree that we shouldn't mention Tanstack Query here.
  5. I think it's a lot easier if we completely rewrite this page in a single PR. In my opinion, this page should be structurally changed. Currently, you have structured this page based on whether data loading happens on client or server. I think we should structure it based on whether we want to use createAsync and its ecosystem or createResource.

That's my opinion. I respect what you have done (and are doing) with the docs. But I think my vision is different. I've tried to convey my vision through my original PR. I would love to help you with this if we can find a middle ground.

I believe a lot of the material in my original PR can be used to tackle the first task (Decoupling server/client data-loading. Putting preload into its own section). There are form submission and Suspense examples but I think I should expand on them a little.

I don't know much about single-flight mutations. But if you point me in the right direction (an example, an article somewhere or even a Discord thread) I can figure it out.

@amirhhashemi
Copy link
Contributor Author

amirhhashemi commented Dec 20, 2024

I just noticed the parent section of Data Loading is called "Building your application". So maybe we should mention Tanstack Query. It's a valid option for building applications with SolidStart. Just a thought.

@atilafassina
Copy link
Member

I see what you're coming from.
I think that generally the title of this section and positioning within the structure is misleading and made us have different expectations towards it.

My initial take was that we should explain how data loading works within the SolidStart context instead of "getting started" chapter about data loading. But you made relevant points and I'm reevaluating my take.

You're idea is very compelling (having a "all about data loading" kind of entry - it's a huge undertake though, if you're up for it, I'd love to help you see this through and then I think we can forget about this existing entry, create a Guides section for SolidStart and add both a "Fetching Data" and a "Mutating Data" entries.

AFAIK, createAsync is a wrapper around createResource but it comes with its ecosystem and the ecosystem is not compatible with createResource.

Yes and no. createAsync is based on createResource and they mostly work the same, the difference is that the fetcher is reactive, which makes createAsync work consistently with query() whereas createResource will occasionally not revalidate/retrigger properly. That being said: createAsync can be used always as a replacement for createResource, but not the other way around. The biggest "problem" we have in documenting that within the boundaries of /solid-start is that createResource is a SolidJS method and createAsync is a solid-router method. But I think @LadyBluenotes and I would agree that those boundaries are more flexible within the Learn subsection, we still like to increase visibility about which methods come from where when possible.

I believe a lot of the material in my original PR can be used to tackle the first task

If I remember right, the case for closing that PR wasn't about the content itself, but that we'd like to have this discussion (again, our fault for not replying to the issue earlier). So the content should still be useful.

I don't know much about single-flight mutations

if you decide to document, I can help you further. But for now, just so I don't leave you hanging:
Single-Flight mutations is a mechanism that exists thanks to SolidStart's internal caching. When a preload function is declared, SolidStart becomes aware of the data for each route. This allows the system to (whenever a mutation happens) to revalidate the data-fetching query during the same roundtrip and return the response with the mutation already with the updated data.

This is an evolution on the mutation system for other frameworks, where every mutation has 2 flights (one for triggering the mutation and another for fetching the revalidated query).


Bottom line is: how about we start a new section to document Data Fetching in a Guide for SolidStart and then once we merge the first guide ? If the information proves to be redundant to what we have in the "build your application" section, we can eliminate that one.

My one and only request for this would be: try not to go too deep into explaining APIs, instead I'd suggest either adding a callout or a link to suggesting further reading in the Reference. That allows to keep the guide short, strengthens internal linking between the sections - which not only helps SEO, but also our internal search (and the AI feature that we need to bring back)

@amirhhashemi
Copy link
Contributor Author

amirhhashemi commented Dec 20, 2024

Developers coming from React probably would not read the Solid Router docs at all, because in the React ecosystem meta-frameworks come with their router. They might map that mentality to SolidStart and think: SolidStart has its router and Solid Router is like react-router for React and it's not used in SolidStart.

Although createAsync and query and other Solid Router primitives are directly imported from @solidjs/router, I don't think most people coming from React read the Solid Router docs before starting SolidStart.

Another mentality of people coming from React is that Data Fetching is the responsibility of the meta-framework or an external library like Tanstack Query and Solid should have nothing to do with it.

I genuinely think most people coming from React start right off from SolidStart. That's my theory.

The point is that they might not internalize it well that createResource is a Solid primitive, createAsync is a Solid Router primitive, and the route object we return from route files is related to Solid Router.

We have three options:

  1. Make it absolutely clear where each primitive comes from and encourage the readers to read the related docs first.
  2. Assume they don't read anything but the SolidStart docs.
  3. The middle ground. While more clearly explaining the relationship between different components used in SolidStart, write the docs so that if they didn't read the related docs first, they at least understand what's going on.

You tried to take the first approach with the caveat that you didn't make the relationship very clear. I think that's why I struggled a lot with SolidStart at first. The relationship between SolidStart and Solid Router was not very clear to me.

I tried to take the second approach.

But I think the third approach is the best and a Guide section would be a great home for that sort of content.

I'm absolutely down to write a "Data Fetching" document in a "Guide" section. Then we can decide whether to keep the current Data Loading page. Even then, if you decide that the Guide section is redundant, I'm fine with it.

If I need to know anything before starting, please let me know.

@atilafassina
Copy link
Member

Although createAsync and query and other Solid Router primitives are directly imported from @solidjs/router, I don't think most people coming from React read the Solid Router docs before starting SolidStart.

Another mentality of people coming from React is that Data Fetching is the responsibility of the meta-framework or an external library like Tanstack Query and Solid should have nothing to do with it.

I genuinely think most people coming from React start right off from SolidStart. That's my theory.

Yes, that's usually it. While we try not to cater for a specific migration/onboarding path, we can't pretend we don't see the fact that most Solid devs are immigrants from React.

The point is that they might not internalize it well that createResource is a Solid primitive, createAsync is a Solid Router primitive, and the route object we return from route files is related to Solid Router.

Some experienced Solid devs don't realize that, to be honest.

But I think the third approach is the best and a Guide section would be a great home for that sort of content.

I'm absolutely down to write a "Data Fetching" document in a "Guide" section. Then we can decide whether to keep the current Data Loading page. Even then, if you decide that the Guide section is redundant, I'm fine with it.

I agree. And more likely the other section will be redundant. I really like where this is going.
I'll take the liberty to assign this issue to you and you can get started. I'll touch base with @LadyBluenotes as soon as Canada is up so she knows what's going on.

Besides that, the only thing I'd recommend you to know before getting started is the WRITING.md in the root.
We have 2 components <Callout> and <TabCodeBlocks> that will help you get a better UX. Other than that, I rather iterate instead of micromanaging. Do what you think is best :)

@atilafassina
Copy link
Member

oh oh oh... can't believe I hadn't thought of it earlier!

@OrJDev has written an amazing "all things data loading" on SolidStart

@amirhhashemi, I totally recommend that as a source

https://github.com/OrJDev/solidstart-data

@devagrawal09
Copy link
Contributor

devagrawal09 commented Dec 20, 2024

I just left a comment here in #914 with a potential restructuring of the data loading docs, would love to hear your thoughts!

@amirhhashemi amirhhashemi linked a pull request Dec 21, 2024 that will close this issue
2 tasks
@amirhhashemi
Copy link
Contributor Author

I published the first draft of the Data Fetching guide. Would love to know your thoughts @atilafassina @devagrawal09

#991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending review Awaiting review by team members.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@atilafassina @devagrawal09 @amirhhashemi @LadyBluenotes and others