Skip to content

Conversation

@rorticus
Copy link
Contributor

This is a copy of #341 but that has been updated to address the requested feedback.

If this is merged, we can close #341.

@rorticus rorticus requested review from agubler, matt-gadd and sbinge July 19, 2019 15:39
Copy link
Member

@agubler agubler left a comment

Choose a reason for hiding this comment

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

A few bits are missing or outdated:

  1. Widgets should be converted to the new function-based API
  2. The stores middleware is not mentioned at all (for use with function-based widgets)


Dojo is a reactive architecture concerned with constantly modifying and rendering the current state of an application. In simple systems this can happen at a local level and widgets can maintain their own state. However, as a system becomes more complex the need to better compartmentalize and encapsulate data and create a clean separation of concerns quickly grows.

Stores provide a clean interface for storing, modifying, and retrieving data from a global object using a unidirectional data flow. Common patterns such as asynchronous retrieval, middleware, and undo patterns are built in. This allows widgets to remain focused on their primary roles of providing a visual representation of themselves and listening for user interactions.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the wording of "asynchronous retrieval"


Adds a value to an object or inserts it into an array.

> `add` example
Copy link
Member

Choose a reason for hiding this comment

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

I think this formatting is used to indicate the filename not a title, perhaps @sbinge can confirm?

];
});

export const login = createProcess('login', [ fetchUser, loadUserData ]);
Copy link
Member

Choose a reason for hiding this comment

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

Should we explain this string "id" now as it is part of the example?

export const login = createProcess('login', [ fetchUser, loadUserData ]);
```

#### Middleware
Copy link
Member

Choose a reason for hiding this comment

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

Do you think middleware is basic usage? I'm not sure...


## Widgets and Stores

There are two state containers available for widgets: `Container` and `StoreProvider`. These containers connect the application store with a widget.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's StoreContainer rather than Container

> widget/User.container.ts

```ts
import { createStoreContainer } from '@dojo/framework/stores/StoreInjector';
Copy link
Member

Choose a reason for hiding this comment

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

This should be imported from StoreContainer


In this example `UserContainer` wraps `User` to display the current user's name. `createStoreContainer` is wrapper that is used to ensure the proper typing of `getProperties`. `getProperties` is responsible for getting to data from the store and creating properties for the widget.

The most important thing to note about a `Container` is its properties are a 1:1 mapping to the widget it wraps. In other words, all properties that appear on the widget should be provided by the `Container`.
Copy link
Member

Choose a reason for hiding this comment

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

The widget properties become the properties of the container, but they are all optional..


## Pre-typing

A pre-typed container can be created by extending the standard `StoreProvider` and passing the `State` type as a generic.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should really recommend doing this? Soon these will be converted to functional widgets and it won't be possible - at that point we'd be able to provide a util function that can be used to create a "typed" provider.


### ImmutableState

Dojo Stores provide an implementation of the MutableState interface that leverages [Immutable](). This implementation may provide better performance if there are frequent, deep updates to the store's state. Performance should be tested and verified before fully committing to this implementation.
Copy link
Member

Choose a reason for hiding this comment

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

needs a link to immutable?

});
```

# JSON Patch and Store operations
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention JSON patch?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this whole section is required? What do other people think?

Copy link
Member

Choose a reason for hiding this comment

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

I think JSON Patch is cool, so to me it's worth mentioning. :)

Copy link
Member

Choose a reason for hiding this comment

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

I would say though, although it's based on JSON patch it's not implemented to exact spec - also it's something that we could change i.e. an implementation detail

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should mention JSON patch at all. It is 100% an implementation detail and we do not let the user write/pass JSON patch anywhere. As @agubler said also we do not fulfil the entire spec (nor do we intend to).

Copy link
Member

@dylans dylans left a comment

Choose a reason for hiding this comment

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

I still need to review the supplemental content and later, but wanted to provide what I've reviewed thus far.

@@ -0,0 +1,454 @@
# Basic Usage

Dojo is a reactive architecture concerned with constantly modifying and rendering the current state of an application. In simple systems this can happen at a local level and widgets can maintain their own state. However, as a system becomes more complex the need to better compartmentalize and encapsulate data and create a clean separation of concerns quickly grows.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Dojo is a reactive architecture concerned with constantly modifying and rendering the current state of an application. In simple systems this can happen at a local level and widgets can maintain their own state. However, as a system becomes more complex the need to better compartmentalize and encapsulate data and create a clean separation of concerns quickly grows.
Dojo provides a reactive architecture concerned with constantly modifying and rendering the current state of an application. In simple systems this can happen at a local level and widgets can maintain their own state. However, as a system becomes more complex the need to better compartmentalize and encapsulate data and create a clean separation of concerns quickly grows.


Dojo is a reactive architecture concerned with constantly modifying and rendering the current state of an application. In simple systems this can happen at a local level and widgets can maintain their own state. However, as a system becomes more complex the need to better compartmentalize and encapsulate data and create a clean separation of concerns quickly grows.

Stores provide a clean interface for storing, modifying, and retrieving data from a global object using a unidirectional data flow. Common patterns such as asynchronous retrieval, middleware, and undo patterns are built in. This allows widgets to remain focused on their primary roles of providing a visual representation of themselves and listening for user interactions.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Stores provide a clean interface for storing, modifying, and retrieving data from a global object using a unidirectional data flow. Common patterns such as asynchronous retrieval, middleware, and undo patterns are built in. This allows widgets to remain focused on their primary roles of providing a visual representation of themselves and listening for user interactions.
Stores provide a clean interface for storing, modifying, and retrieving data from a global object through unidirectional data flow. Stores include support for common patterns such as asynchronous data retrieval, middleware, and undo. Stores and their patterns allow widgets to focus on their primary role of providing a visual representation of information and listening for user interactions.


## The Store

The store holds the global, atomic state for the entire application. It is created when the application is created and defined in the `Registry` with an injector.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The store holds the global, atomic state for the entire application. It is created when the application is created and defined in the `Registry` with an injector.
The store holds the global, atomic state for the entire application. The store gets created when the application gets created and defined in the `Registry` with an injector.

const registry = registerStoreInjector(store);
```

`State` defines the shape of the global store using an interface. It is important that everything inside `State` is serializable, as this improves performance and makes it easier for Dojo to determine when changes to the data occur.
Copy link
Member

Choose a reason for hiding this comment

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

We've used the term global a lot, and in general in JS, global means global variable, rather than global to the application, so we may want to be more precise about that term throughout.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the term shape will make a lot of sense to a new user at this point?

Copy link
Member

Choose a reason for hiding this comment

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

Similarly, will someone know at this point what it means to make something serializable?

const registry = registerStoreInjector(store);
```

`State` defines the shape of the global store using an interface. It is important that everything inside `State` is serializable, as this improves performance and makes it easier for Dojo to determine when changes to the data occur.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`State` defines the shape of the global store using an interface. It is important that everything inside `State` is serializable, as this improves performance and makes it easier for Dojo to determine when changes to the data occur.
`State` defines the shape of the global store using an interface. Everything inside `State` should be serializable, as this improves performance by making it easier for the Dojo virtual DOM system to determine when changes to the data occur.

I don't know this suggestion is accurate, but something like this (plus address my other comments please)


### Excuting a Process

A process simply defines an execution flow for a set of data. To execute a process it needs access to the store to create an executor. Both the `Container` and `StoreProvider` widgets will provide you with access to the store.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A process simply defines an execution flow for a set of data. To execute a process it needs access to the store to create an executor. Both the `Container` and `StoreProvider` widgets will provide you with access to the store.
A process simply defines an execution flow for a set of data. To execute a process, the process needs access to the store to create an executor. Both the `Container` and `StoreProvider` widgets provide access to the store.

@@ -0,0 +1,15 @@
# Introduction

Dojo stores is a predictable, consistent state container with built-in support for common patterns.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Dojo stores is a predictable, consistent state container with built-in support for common patterns.
Dojo stores provide a predictable, consistent state container with built-in support for common state management patterns.


Dojo stores is a predictable, consistent state container with built-in support for common patterns.

This package provides a centralized store designed to be the single source of truth for an application. It operates using uni-directional data flow. This means all application data follows the same lifecycle, ensuring the application logic is predictable and easy to understand.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This package provides a centralized store designed to be the single source of truth for an application. It operates using uni-directional data flow. This means all application data follows the same lifecycle, ensuring the application logic is predictable and easy to understand.
This Dojo stores package provides a centralized store designed to be the single source of truth for an application. Dojo applications operate using uni-directional data flow. As a result, all application data follows the same lifecycle, ensuring the application logic is predictable and easy to understand.


| Feature | Description |
| ------------------------- | -------------------------------------------------------------------- |
| global data store | application state is stored globally in a single source of truth |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| global data store | application state is stored globally in a single source of truth |
| global data store | application state gets stored globally in a single source of truth |

again I would somehow look to make it clear that this is global to the app and not a JS global without adding a bunch of words

| ------------------------- | -------------------------------------------------------------------- |
| global data store | application state is stored globally in a single source of truth |
| uni-directional data flow | predictable and global application state management |
| type safe | access and modification of state is protected by interfaces |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| type safe | access and modification of state is protected by interfaces |
| type safe | access and modification of state gets protected by interfaces |

Copy link
Member

@dylans dylans left a comment

Choose a reason for hiding this comment

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

And some more suggestions.

@@ -0,0 +1,425 @@
# State

In modern browsers a `state` object is passed as part of the CommandRequest. Any modification to this object is translated to the appropriate patch operations and applied against the store.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In modern browsers a `state` object is passed as part of the CommandRequest. Any modification to this object is translated to the appropriate patch operations and applied against the store.
In modern browsers a `state` object is passed as part of the `CommandRequest`. Any modification to this `state` object gets translated to the appropriate patch operations and applied against the store.

});
```

Note that attempting to access state is not supported in IE and will immediately throw an error.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that attempting to access state is not supported in IE and will immediately throw an error.
Note that attempting to access state is not supported in IE11 and will immediately throw an error.


The StoreProvider accepts three properties

- `renderer`: A render function that has the store injected in order to access state and pass processes to child widgets.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `renderer`: A render function that has the store injected in order to access state and pass processes to child widgets.
- `renderer`: A render function that has the store injected to access state and pass processes to child widgets.


The `StoreProvider` has two main ways to trigger invalidation and cause a rerender.

1. The recommended approach is to register `path`s by passing the `paths` property to the provider to ensure invalidation will only occur when state you are interested in changes.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. The recommended approach is to register `path`s by passing the `paths` property to the provider to ensure invalidation will only occur when state you are interested in changes.
1. The recommended approach is to register `path`s by passing the `paths` property to the provider to ensure invalidation only occurs when relevant state changes.

/>
```

If you are **not** using `tsx`, in order for TypeScript to infer this correctly when using w(), the generic will need to be explicitly passed.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you are **not** using `tsx`, in order for TypeScript to infer this correctly when using w(), the generic will need to be explicitly passed.
If you are **not** using `tsx`, in order for TypeScript to infer this correctly when using `w()`, the generic needs to get passed explicitly.

});
```

# JSON Patch and Store operations
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# JSON Patch and Store operations
# JSON Patch and Store Operations


> Result

```json
Copy link
Member

Choose a reason for hiding this comment

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

Most of our other guide examples use spaces for indentation, so we may want to change this for consistency?


Even though state has not been initialized, Dojo is able to create the underlying hierarchy based on the provided path. This operation is safe because of the type safety that TypeScript and Dojo Stores provide. This allows for users to work naturally with the `State` interface used by the store instead of needing to be concerned with the data explicitly held by the store.

When an explicit state is required that information can asserted by using a `test` operation or by getting the underlying data and validating it programmatically.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When an explicit state is required that information can asserted by using a `test` operation or by getting the underlying data and validating it programmatically.
When an explicit state is required, that information can get asserted by using a `test` operation or by getting the underlying data and validating it programmatically.

]);
```

This example ensures that `user` is always added to the end of the list as the last element.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This example ensures that `user` is always added to the end of the list as the last element.
This example ensures that `user` always gets added to the end of the list as the last element.

]);
```

Access to state root is not permitted and will throw an error, for example, `get(path('/'))`. This applies to Operations also, it is not possible to create an operation that will update the state root. Best practices with @dojo/framework/stores mean touching the smallest part of the store as is necessary.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Access to state root is not permitted and will throw an error, for example, `get(path('/'))`. This applies to Operations also, it is not possible to create an operation that will update the state root. Best practices with @dojo/framework/stores mean touching the smallest part of the store as is necessary.
Access to state root is not permitted and will throw an error, for example, `get(path('/'))`. This restriction also applies to Operations; it is not possible to create an operation that will update the state root. Best practices with `@dojo/framework/stores` encourage accessing the smallest necessary part of the store.

@rorticus
Copy link
Contributor Author

@agubler @dylans I believe all of the feedback has been addressed!


## The Store

The store holds the global, atomic state for the entire application. The store gets created when the application gets created and defined in the `Registry` with an injector.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe say "The store should be created..." instead of "The store gets created...". I feel like the latter makes it sound like this will happen automatically.


### Commands & Operations

To modify a store, when executing a process, an operation function gets invoked. The operation function gets returned from a command. Each command is passed a `CommandRequest` which provides `path` and `at` functions to generate `Path`s in a type-safe way, a `get` function for access to the store's state, a `payload` object for the argument that the process executor was called with.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this sentence say "command function" instead of "operation function"? And in the next sentence, "The operation function gets returned from a command", is that right? Commands return an array of operations (or a promise or nothing), not a function.


In this example `UserContainer` wraps `User` to display the current user's name. `createStoreContainer` is a wrapper that gets used to ensure the proper typing of `getProperties`. `getProperties` is responsible for accessing data from the store and creating properties for the widget.

A `StoreContainer`'s properties are a 1:1 mapping to the widget it wraps. The widget become the properties of the `StoreContainer`, but they are all optional.
Copy link
Contributor

Choose a reason for hiding this comment

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

"The widget become the properties" I am assuming this means something like The widget is passed the properties or The widget receives the properties? If this isn't a mistake maybe some more clarification is in order?

@rorticus
Copy link
Contributor Author

@maier49 I believe your feedback has been addressed!

@sbinge sbinge added the reference guide documentation label Jul 30, 2019
Copy link
Contributor

@sbinge sbinge left a comment

Choose a reason for hiding this comment

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

We can get this in as-is with any further changes raised later as follow-up PRs, if needed.

@sbinge sbinge merged commit 3015e46 into dojo:master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reference guide documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants