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

docs: read updates #2

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

docs: read updates #2

wants to merge 7 commits into from

Conversation

puppybits
Copy link
Contributor

Description

Read documentation updates. Please call out anything that's unclear in the documentation.

@puppybits puppybits requested a review from a team April 6, 2022 19:08

```ts
// useRead sets up a global subscription for changes &
// will rerender when items are added/removed but NOT if a doc's values change

Choose a reason for hiding this comment

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

I get a little confused here.

  • If task name in task-294 changes, it won't be re-rendered

  • if a new task is included in Firestore, then it will re-render

correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this code if TASK-294 changes the title only the listItem for TASK-294 will be pre-rendered. This is because we use the list parent only gets an array id & path. That doesn't change so no render happens. Each item component also doesn't have any changes so only the component with the new title will pre-render.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a new task is add (or the order of the tasks is changed) then the parent will re-render. When it re-renders the key, id and path are the same for all the existing component so they will not re-render. Only the new item will be added and rendered in React.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is why the useRead and the useCache are so powerful. We can skip huge amount of React dirty checks and React renders because useRead and useCache can do a shallow equality check to skip React checks. When we only pass props to a component that are primitives then React is do a simple equality check and skip all expensive dirty checks and re-renders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@victorodgs is there a better way we can say this?

Choose a reason for hiding this comment

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

Would it be better to say we should only render a list from IDs and then useCache at a lower level for display data like title?

Copy link
Member

@comlaterra comlaterra left a comment

Choose a reason for hiding this comment

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

Just a comment to references to render. Let me know what you think

docs/read.md Outdated

In Read/Write subscriptions are turned on by default. Anytime useRead is
called it automatically listens for any document changes both locally and
global and will rerender when data changes. As of v1, there is not a single
Copy link
Member

Choose a reason for hiding this comment

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

Since rendering is not part of this library, I think it is important to distinguish between rendering and updating the data. In this case, I would say that the component that read is then listening to any data changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, our library manages data loading and updates. But the reader are all using it inside their React component.

So the doc should be written to who the reader is. In this case it's a UI engineer wanting to know how it will affect their rendering.

For the deep dive docs their purpose is for a library maintainer. So those show go deeper into when the data updates, where the strict vs shallow vs deep equality checks are and how they will impact the callee which is a React component and if it will do a costly dirty check.

global and will rerender when data changes. As of v1, there is not a single
fetching; only a fetch and subscribe for global changes.

You can minimize when rerender are triggered by selecting a
Copy link
Member

Choose a reason for hiding this comment

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

Same here, as I see it, what you are minimizing is the amount of data you are listening to.

docs/read.md Outdated Show resolved Hide resolved
global and will rerender when data changes. As of v1, there is not a single
fetching; only a fetch and subscribe for global changes.

You can minimize when rerender are triggered by selecting a
Copy link
Contributor Author

@puppybits puppybits Apr 7, 2022

Choose a reason for hiding this comment

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

@comlaterra

Suggested change
You can minimize when rerender are triggered by selecting a
Reducing the data requested will reduce the updates to the hook. This will
have the effect of reducing renders of your React component. Reduce updates
and re-renders with a


```ts
const archiveTask = createMutate({
const archiveAction = createMutate({
action: 'ArchiveTask',

read: (taskId) => ({ taskId: () => taskId }),

Choose a reason for hiding this comment

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

Why do we need this line?

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.

5 participants