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

Add Data Fetching Guide #991

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

amirhhashemi
Copy link
Contributor

@amirhhashemi amirhhashemi commented Dec 21, 2024

  • I have read the Contribution guide
  • This PR references an issue (except for typos, broken links, or other minor problems)

Description

The idea of the Guides section is to provide a resource for developers who don't read Solid.js and Solid Router docs before learning SolidStart, such as React developers who may think of SolidStart as what Next.js is for React and start learning Solid.js from SolidStart. However, I think a Guides section can be useful to a lot of people regardless of whether they come from React.

The goal is to get those users (or anyone really) up and running with SolidStart as quickly as possible, while also guiding them to read more relevant docs if they want to learn more.

This PR adds a Data Fetching page under the Guides section that covers basic data fetching patterns while providing links to relevant documentation for those who want to learn more.

See #861 for more context.

Related issues & labels

#914 #861

Copy link

stackblitz bot commented Dec 21, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

netlify bot commented Dec 21, 2024

Deploy Preview for solid-docs ready!

Name Link
🔨 Latest commit 9dd327d
🔍 Latest deploy log https://app.netlify.com/sites/solid-docs/deploys/6776c3fdafe8d50008bc67e9
😎 Deploy Preview https://deploy-preview-991--solid-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@LadyBluenotes
Copy link
Member

LadyBluenotes commented Dec 21, 2024

Hi @amirhhashemi,

First of all, thank you for taking the time to work on this, I appreciate the time you're taking out of your day to improve our documentation.

It might be useful to take a look at the diataxis documentation on how guides should be written. We use this as a reference for writing our docs, and i feel like it could be helpful to consider for this!

If you'd like some more directed feedback, however, please let me know

@amirhhashemi
Copy link
Contributor Author

Hi @LadyBluenotes

Thank you for the feedback. I made some changes based on the diataxis guide:

  1. I removed all explanations that are more suited for a tutorial page and only relied on links to provide more info.
  2. I tried to make each section more action-oriented. Now, the titles start with "How to" and the minimal description of each section starts with "To do".
  3. I reordered the sections to create a better flow.

I'm not sure about:

  1. Whether I should keep the examples self-contained. I think it's a good thing because readers can pick up each section and copy its example without needing to read the previous sections.
  2. Whether the section about Tanstack Query belongs on this page. Maybe it's more suited for a tutorial page or the "build your application" section.

@LadyBluenotes
Copy link
Member

Hi @LadyBluenotes

Thank you for the feedback. I made some changes based on the diataxis guide:

1. I removed all explanations that are more suited for a tutorial page and only relied on links to provide more info.

2. I tried to make each section more action-oriented. Now, the titles start with "How to" and the minimal description of each section starts with "To do".

3. I reordered the sections to create a better flow.

I'm not sure about:

1. Whether I should keep the examples self-contained. I think it's a good thing because readers can pick up each section and copy its example without needing to read the previous sections.

2. Whether the section about Tanstack Query belongs on this page. Maybe it's more suited for a tutorial page or the "build your application" section.

Thank you for adding some changes! One comment I do want to make, is that I think adding back some (basic) explanations can be helpful. Doesn't have to explain anything but the process, such as this:

To show users that data is loading, you can display a loading UI while they wait. You can achieve this by wrapping your component in Solid's <Suspense> component and using the fallback prop to specify what should be shown during the loading process. Here's how:

  1. Import the component from SolidJS.
  2. Wrap your component with and define a fallback UI to display while waiting for the data to load.

This way it follows we have the code snippet explained a bit in case users need some kind of explanation (but not giving too much background about the topic itself, which is more appropriate for other sections in the docs). This is a pretty rough example but we can either have it as a quick list (if you feel it's necessary) or just a simple explanation of how things are set up in the snippet.

I do agree that, to an extent, having each section self-contained is important. We don't want them so far removed that they could be considered separate entirely, but enough that there is some relationship (even if marginally).

As for the Tanstack Query section, I do agree that it would probably be better suited for a tutorial than in here. That being said, a callout could probably replace the section saying something as simple as what you wrote for the first sentence:

Tanstack Query can be used for more advanced features such as automatic background re-fetching or infinite queries.

Do you have any questions/ comments about what I mentioned?

@amirhhashemi
Copy link
Contributor Author

Thank you for the feedback.

I've added basic explanations for each section. However, I have some concerns:

  1. Some explanations may be redundant and could be streamlined
  2. I'm particularly uncertain about the createResource section - it's challenging to explain without diving too deep into its internal workings

I would appreciate your feedback on these points. Please feel free to edit the document directly if needed.

Regarding Tanstack Query: I've removed the dedicated section and added a brief note about it at the end of the createResource section instead.

@amirhhashemi
Copy link
Contributor Author

I'm wondering whether we should remove the "how to" from section titles:

  • How to handle errors -> Handling errors
  • How to preload data -> Preloading data
    etc

The way this guide is written, it's obvious we are only talking about how to do stuff, and not why and when. I think we can remove it. What do you think?

@LadyBluenotes
Copy link
Member

@amirhhashemi Thank you for adding the explanations! I'll try and get to reviewing them this weekend (and seeing how we can approach the createResource section) but I wanted to say I think it's a great Idea to update the titles to remove the how to parts!

Copy link
Member

@LadyBluenotes LadyBluenotes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know how you feel about these changes, but I feel like you did a great job explaining each code snippet briefly.

src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
src/routes/solid-start/guides/data-fetching.mdx Outdated Show resolved Hide resolved
kodiakhq bot and others added 13 commits December 30, 2024 04:58
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
Co-authored-by: Sarah <gerrardsarah@gmail.com>
@amirhhashemi amirhhashemi marked this pull request as ready for review December 30, 2024 08:15
@amirhhashemi
Copy link
Contributor Author

amirhhashemi commented Dec 30, 2024

I really appreciate your feedback and help @LadyBluenotes. I think this PR if ready for the final review.

I can work on a data mutation guide in another PR if you like. I just went for it:) #994

@amirhhashemi
Copy link
Contributor Author

amirhhashemi commented Dec 30, 2024

After this is merged, I see no reason for the current Date Loading page to remain. Should I remove it @LadyBluenotes?

Edit: I created a PR that removes this page #995

@amirhhashemi amirhhashemi mentioned this pull request Jan 1, 2025
2 tasks
Comment on lines +192 to +214
When you only need to fetch data on the client side, you can use the [`createResource`](/reference/basic-reactivity/create-resource) primitive:

```tsx {4-7} title="src/routes/index.tsx"
import { createResource, ErrorBoundary, Suspense, For } from "solid-js";

export default function Page() {
const [posts] = createResource(async () => {
const posts = await fetch("https://my-api.com/posts");
return await posts.json();
});
return (
<ul>
<ErrorBoundary fallback={<div>Something went wrong!</div>}>
<Suspense fallback={<div>Loading...</div>}>
<For each={posts()}>{(post) => <li>{post.title}</li>}</For>
</Suspense>
</ErrorBoundary>
</ul>
);
}
```

You can [read more about `createResource` here](/reference/basic-reactivity/create-resource).
Copy link
Contributor

@devagrawal09 devagrawal09 Jan 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So createAsync can still be used to fetch on the client side, and the only reason you'd use createResource instead is if you're not using solid-router, not if you're fetching on the client. This section seems to confuse those two things together.

I'd break this into two separate sections, one that shows fetching on the client with createAsync without "use server", and another that shows fetching using createResource on both client and server.

Nevermind, I just noticed that most of your createAsync snippets are fetching on the client already. The only correction I have is this section should be called "Fetching data without Solid Router" or something like that.

Copy link
Contributor Author

@amirhhashemi amirhhashemi Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the diataxis guide that we use as reference:

How-to guides must be written from the perspective of the user, not of the machinery. A how-to guide represents something that someone needs to get done. It’s defined in other words by the needs of a user. Every how-to guide should answer to a human project, in other words. It should show what the human needs to do, with the tools at hand, to obtain the result they need.

This is in strong contrast to common pattern for how-to guides that often prevails, in which how-to guides are defined by operations that can be performed with a tool or system. The problem with this latter pattern is that it offers little value to the user; it is not addressed to any need the user has. Instead, it’s focused on the tool, on taking the machinery through its motions.

Keeping each section action-oriented, focusing on a practical action that the user might want to do offers better value to the user. If the user wants to learn about a specific tool, it can go to the API Reference.

I believe the documentation should include a section discussing the pros and cons of using Solid.js versus Solid Router APIs for data fetching. Additionally, it would be helpful to clarify why some APIs are part of Solid Router while others belong to Solid.js. However, this page is not aimed at that type of content. Perhaps a tutorial on data fetching would be a more appropriate format for this discussion.

Copy link
Contributor Author

@amirhhashemi amirhhashemi Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my initial draft, I included a call-out that gave a little hint:

<Callout type="info">
  Note that `query` and `createAsync` are imported from `@solidjs/router`. By default, SolidStart uses [Solid Router](https://docs.solidjs.com/solid-router) under the hood. That means we can take advantage of all Solid Router data fetching primitives in SolidStart.
</Callout>

This may slightly disrupt the page's theme, but I think reintroducing a call-out like this could significantly help users navigate the material with greater clarity. At least until we add a comprehensive tutorial.

Comment on lines +216 to +218
<Callout type="info" title="Advanced Data Handling">
For advanced features like automatic background re-fetching or infinite queries, you can use [Tanstack Query.](https://tanstack.com/query/latest/docs/framework/solid/overview)
</Callout>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the callout, however I still think it would be helpful to show an example snippet here that shows using tanstack query with "use server". Would love to get some more thoughts on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost wonder if we need a tutorial geared towards advanced data fetching. Like something that goes into the weeds of the more complicated parts people may run into?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think showcasing the usage of Tanstack Query with "use server" is interesting enough to include in this guide. But, my concern is that it might go too far. If we want to cover Tanstack Query properly, we should add:

  1. A basic example of createQuery (with and without "use server")
  2. A section about preloading data
  3. A section about error handling and loading UI
  4. A few examples of advanced things that Tanstack Query can do and SolidStart can't

It's too much for this page. I think it's better to leave it to the user to go through the Tanstack Query docs and figure things out. If there's any ambiguity regarding the use of "use server" with Tanstack Query, we should consider updating other sections of the documentation for clarity.

Comment on lines +90 to +121
## Preloading data

Data fetching can be optimized during user navigation by preloading the data with the [`preload`](/solid-router/reference/preload-functions/preload) function:

1. Export a `route` object with a `preload` function
2. Run your query inside the `preload` function
3. Use the query as usual in your component

```tsx {9-11} title="src/routes/index.tsx"
import { ErrorBoundary } from "solid-js";
import { query, createAsync, type RouteDefinition } from "@solidjs/router";

const getPosts = query(async () => {
const posts = await fetch("https://my-api.com/posts");
return await posts.json();
}, "posts");

export const route = {
preload: () => getPosts(),
} satisfies RouteDefinition;

export default function Page() {
const post = createAsync(() => getPosts());
return (
<div>
<ErrorBoundary fallback={<div>Something went wrong!</div>}>
<h1>{post().title}</h1>
</ErrorBoundary>
</div>
);
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we explain why preloading is even needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more worthwhile having that in a the other aspects of the docs (like the core parts of "learn" or a "tutorial". This is geared more towards people asking "how to" instead of "why" and i worry if we add the "why" here, it'd be assumed the rest of it should have it as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense, we probably need a separate document for explaining the concepts and issues around data fetching + mutation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants