Skip to content

Conversation

Shrugsy
Copy link
Collaborator

@Shrugsy Shrugsy commented May 2, 2021

Adds a 'Cached data' docs page. Intended to cover as much of the caching concept in a single page as is reasonable.

Moves the 'invalidation' stuff from the 'Mutation' page onto this 'Cached data' page, as IMO adding more details about invalidation would have cluttered and distracted too much on the 'Mutation' page.

Note regarding the 'Invalidation behavior' table:
The colour for the 'diagonal' line in the upper left table cell for 'invalidation behaviour' is hard coded in.
I couldn't find a great way to have a diagonal line in the cell like that using css and use a css variable for the colour (--ifm-table-border-color is what we would have wanted).
Since the best method I found uses an inline svg in a background: url(...) string, I couldn't see a way to use the css variable for the colour.
However, hard-coding the colour from the light-mode border doesn't look extremely out of place in dark mode IMO

image

image

@netlify
Copy link

netlify bot commented May 2, 2021

Deploy preview for redux-starter-kit-docs ready!

Built with commit 2828b7d

https://deploy-preview-1036--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 2, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2828b7d:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

I would change the wording of the first few paragraphs a bit.

In my mind, it's more like "the cache data for this endpoint provides these tags" not "this endpoint provides these tags to the cache".


## Definitions

- _tags_ - Items used to identify the data present in the cache. An individual `tag` has a `type`, represented as a `string` name, and an optional `id`, represented as a `string` or `number`.
Copy link
Member

Choose a reason for hiding this comment

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

This in combination with the paragraph can be read as "if two endpoints have the same tags and one has been fetched, the other won't be".
Can we make it more clear that tags are just for refetching?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've relocated the definitions from the 'Mutation' page into this 'Cached data' page with some minor tweaks/additions, and added a 'note' section to the 'Providing cache data' section to better clarify the intent & behaviour of tags with a more explicit explanation

})
```

Note that for the example above, the `id` is used where possible on a successful result. In the case of an error, no result is supplied, and we still consider that it has provided the general `'Post'` tag type rather than any specific instance of that tag.
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I'm not 100% sure if providing the general ['Post'] tag makes any sense here. That would invalidate only if an endpoint would invalidate all posts by invalidatesTags: ['Post'].
I'd rather have a LIST id in both results as well.

Copy link
Member

Choose a reason for hiding this comment

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

PS: reading further I why it is still missing here though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah the goal was to try to introduce it as a general concept piece by piece first rather than immediately going into 'This is how you should be using it'.

I've added another 'tip' block in the 'Providing cache data' section linking to the 'Advanced Invalidation' section which hopefully is clear enough.

@Shrugsy Shrugsy marked this pull request as ready for review May 3, 2021 13:22
@Shrugsy Shrugsy changed the title [WIP DRAFT] - Docs - add draft 'Cached data' concept page Docs - add 'Cached data' concept page May 3, 2021
Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

Gonna add some minor changes & then merge this in 👍

@phryneas
Copy link
Member

Ahh damn conflicts. The joys of concurrent PRs 😅

@Shrugsy if you have the resources could you please fix those up and maybe also add TS to the examples here?
Otherwise, I'll try but I probably won't get to it before thursday.

@Shrugsy
Copy link
Collaborator Author

Shrugsy commented May 10, 2021

Ahh damn conflicts. The joys of concurrent PRs

@Shrugsy if you have the resources could you please fix those up and maybe also add TS to the examples here?
Otherwise, I'll try but I probably won't get to it before thursday.

No problem, I'll get it fixed up when I have some time

Shrugsy and others added 10 commits May 11, 2021 20:24
- Add miscellaneous content to the 'Cached Data' docs page
- Fix outdated links to renamed `provides`/invalidates` docblock summaries
- Expand `providesTags` docblock summary
- Add tag invalidation examples & table
Co-authored-by: Lenz Weber <mail@lenzw.de>
- Relocate cache invalidation sections from `Mutations` page to `Cached Data`
- Fix incorrect 'invalidation' examples in 'Invalidation Behavior' table
@Shrugsy Shrugsy force-pushed the docs/add-cache-concept-page branch from 23e241c to 2828b7d Compare May 11, 2021 11:03
@Shrugsy
Copy link
Collaborator Author

Shrugsy commented May 11, 2021

@phryneas I've rebased on top of feature/v1.6-integration, addressed the conflicts, & converted the codeblocks to typescript

@phryneas phryneas merged commit a42a9f2 into reduxjs:feature/v1.6-integration May 11, 2021
@phryneas
Copy link
Member

Thank you!

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.

2 participants