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

First pass at derived state documentation #269

Conversation

MGibson1
Copy link
Member

@MGibson1 MGibson1 commented Jan 5, 2024

Objective

Adding documentation for derived state to the state provider deep dive.

I moved this to a separate file because it felt optional and different enough to warrant some extra separation.

@MGibson1 MGibson1 requested a review from a team as a code owner January 5, 2024 18:42
Copy link

cloudflare-workers-and-pages bot commented Jan 5, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9b8e46e
Status: ✅  Deploy successful!
Preview URL: https://9a7ad5b8.contributing-docs.pages.dev
Branch Preview URL: https://ps-add-derived-state-to-stat.contributing-docs.pages.dev

View logs

docs/architecture/deep-dives/state/derived-state.md Outdated Show resolved Hide resolved
docs/architecture/deep-dives/state/derived-state.md Outdated Show resolved Hide resolved
in application state. Derived state exists to store this kind of data in memory and keep it up to
date when the underlying observable state changes.

## `DeriveDefinition`
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ and it may be too late here, but why not "derived" for the object name? This would coincide with the "derived" state lower down. Or is this a "derivation" definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

The thinking was just that the state has already been done, hence past tense while the definition is describing how to derive.

docs/architecture/deep-dives/state/derived-state.md Outdated Show resolved Hide resolved
- `deps` - dependencies used to instantiate the derived state. These are provided when the
`DerivedState` class is instantiated. This object should contain all of the application runtime
dependencies for transform the from parent state to the derived state.
- `cleanupDelayMs` (optional) - Takes the number of milliseconds to wait before cleaning up the
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ What's the underlying purpose for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is effectively the lifetime of derived values to live in memory once observers unsubscribe.

If you have some data that is expensive to derive, you may want to increase this to minimize the chance that data must be re-calculated.

Copy link
Contributor

@withinfocus withinfocus left a comment

Choose a reason for hiding this comment

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

Makes sense!

@MGibson1 MGibson1 merged commit 7b84808 into add-state-provider-framework-deep-dive Jan 24, 2024
3 checks passed
@MGibson1 MGibson1 deleted the ps/add-derived-state-to-state-provider-deep-dive branch January 24, 2024 19:22
justindbaur added a commit that referenced this pull request Sep 23, 2024
* Add State Provider Framework Deep Dive

* Update Deep Dive

* Apply Suggestions from Matt

Co-authored-by: Matt Gibson <[email protected]>

* Update docs/architecture/deep-dives/state/index.md

Co-authored-by: Matt Gibson <[email protected]>

* More Feedback

* Refine

* Add Diagram and FAQ

* Apply suggestions from code review

Co-authored-by: Matt Bishop <[email protected]>

* Run Prettier

* Stub Out `deserializer` and add comment

Co-authored-by: audrey-jensen <[email protected]>

* Clarify Wording

* First pass at derived state documentation (#269)

* First pass at derived state documentation

* `npm run prettier` 🤖

* Apply suggestions from code review

Co-authored-by: Matt Bishop <[email protected]>

---------

Co-authored-by: Matt Bishop <[email protected]>

* Apply suggestions from code review

Co-authored-by: Matt Bishop <[email protected]>

* Fill In TODOs

* Added optional constructor parameter and updated mapping instructions. (#296)

* Added optional constructor parameter and updated mapping instructions.

* Fixed table

* Changed verbiage.

* Updated verbiage.

* Updated to include clearOn and UserKeyDefinition

* Update `ActiveUserState` Information

* Fix AUS Warning

Co-authored-by: ✨ Audrey ✨ <[email protected]>

* Change to instruction

Co-authored-by: ✨ Audrey ✨ <[email protected]>

* Move to Present Tense

---------

Co-authored-by: Matt Gibson <[email protected]>
Co-authored-by: Matt Bishop <[email protected]>
Co-authored-by: audrey-jensen <[email protected]>
Co-authored-by: Todd Martin <[email protected]>
Co-authored-by: Todd Martin <[email protected]>
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