Skip to content
This repository has been archived by the owner on Oct 11, 2020. It is now read-only.

link to codesandbox demo of useContext example #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

100ideas
Copy link

@100ideas 100ideas commented May 17, 2019

To understand the example code in the 'Accessing stores' recipe, I implemented it in codesandbox. Here is a github repo if you want to fork it: https://github.com/100ideas/mobx-react-usecontext-codesandbox-demo

I was unable to get the original createStore() function to work as an observable, so I changed it slightly to an observable(object). You should review it before accepting this PR to make sure what I changed is how you intend this pattern to be implemented.

In the original example, the createStore() function is never made into an observable (I found this confusing - is it implicit somewhere?), and the function is passed as a reference but not called in the context provider code React.useState(createStore). I may be missing something implied or obvious about the example code, but I couldn't get it to work as written, so I changed it.

100ideas added 2 commits May 17, 2019 06:06
To understand the example code in more detail, I implemented it in codesandbox. Here is a github repo if you want to fork it: https://github.com/100ideas/mobx-react-usecontext-codesandbox-demo

I was unable to get the original `createStore()` function to work as an observable, so I changed it slightly to an `observable(pojo)`. You should review it before accepting this PR to make sure what I changed is how you intend this pattern to be implemented.

```
 createStore() {
  // storing variable in closure means you can access it in methods
  // otherwise you would have to use `this`
  const store = {
//...
```
@danielkcz
Copy link
Collaborator

Funny, I was thinking about doing exactly this today :)

You are right about createStore, it's wrong in the example. It should be like this.

export const StoreProvider = ({ children }) => {
  const store = useLocalStore(createStore)

The way you did it isn't wrong, but I dislike the use of this as it's not always working properly in TypeScript.

That said I would prefer if example in CodeSandbox would be written with TypeScript since the original is like that as well. If you are not feeling up to that, I will make it happen sometimes later. Good stuff either way.

@100ideas
Copy link
Author

I get it about this, I prefer your form as well and often find myself weirdly resistant to the "traditional mobx" pattern of implementing models and stores as classes.

The discussion about the changing API in mobx-react & mobx-react-lite has been moving so fast that I wasn't sure if useLocalStore() was stable or when to consider using it. If that's your recommendation I'll change the code.

I don't really like typescript for prototyping b/c I find it generally slows me down with a bunch of syntax and composite type errors - but this is because I just don't use/understand typescript much. Here is an updated codesandbox with your suggested changes, I am not sure why it isn't working: https://codesandbox.io/s/mobxreact-usecontext-provider-demo-ls076

@100ideas
Copy link
Author

100ideas commented May 17, 2019

Later today I am going to try extending this example into an app that demonstrates how to create multiple stores that manage models that relate to one another. The "model(pojo)-view(react)-controller(mobx)" pattern.

examples of pattern:

relevant discussion: #3 (comment)

Stores are singletons. Root store has an init() method (or just constructor). Root store creates singletons of all sub-stores, passing a reference to itself into the sub-store's constructor or factory. Logic in sub-stores can use this reference to access methods in sibling stores. For instance, pseudocode example of how to delete related content in two stores:

// PostStore is mobx observable object or class instance w/ ref to rootStore
PostStore.deletePost = function( postId ){
  const { commentIds } = PostStore.posts.get( postId )
  PostStore.RootStore.CommentStore.removeByIds( commentIds ) 
  PostStore.posts.remove( postId );
}

If you have any suggestions about how NOT to implement this, or preferred patterns that I could use, please let me know!

@danielkcz
Copy link
Collaborator

danielkcz commented May 18, 2019

Thank you for the effort of converting to TypeScript. I kinda consider myself TypeScript messenger and want to try to spread the word as much as I can so people can see it's nothing to be afraid of. On the contrary, it has already helped me in so many tricky situations that it's definitely worth it.

The example is not working simply because of all files with JSX has to be TSX :) I used to fight with that as well, but it's not that hard-to-learn habit. And the createStore.tsx can be TS instead ;)

A more complex example would be definitely great. Personally, I prefer mobx-state-tree for setting up a complex state, but it's definitely ok to invent some other approaches. I can imagine the approach of defining separate custom hooks, each exporting the result of useLocalStore (or something else) and assembling together to a one state object. The composition is always better than mangling everything into one big file.

The relation between stores would be tough doing as you drafted though. Not sure how to approach that.

The discussion about the changing API in mobx-react & mobx-react-lite has been moving so fast that I wasn't sure if useLocalStore() was stable or when to consider using it. If that's your recommendation I'll change the code.

Since the docs are using it quite heavily all around, it's indeed considered stable :)

@danielkcz
Copy link
Collaborator

I am actually thinking that instead of CodeSandbox I would like to include that example in the site itself in a similar fashion the intro page has.

Thing is that as much as CodeSandbox is great for experimenting, in a year or so you might decide you need to delete that sandbox for some spring cleaning and it breaks the docs.

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

Successfully merging this pull request may close these issues.

2 participants