prodo-dev / prodo

Prodo is a React framework to build apps faster.
https://docs.prodo.dev
MIT License
115 stars 5 forks source link

Feedback #132

Closed mathieudutour closed 4 years ago

mathieudutour commented 4 years ago

@Onurbon asked me to have a look so here are a couple of thoughts:

const App = () => { // setCount would be `dispatch(model.action(state => x => { fnPassedToUseState(state) = x })) behind the scene const [count, setCount] = useState((state: State) => state.count)

// in case useState isn't enough const action = useAction((state: State) => { // do whatever }) return (

{count}

); }

export default App;



You will note a few things:
- import `@prodo/react` instead of a relative path. That means that you can potentially share components using global states as long as they use prodo (could be useful to create a design system, etc.).
- no need to `watch`/`dispatch`, the `useState` automatically returns what's needed (it should be possible as long as the component is nested inside a `Provider`, right?)
mathieudutour commented 4 years ago

Trying it with Gatsby and it works quite nicely.

You need 3 files:

I would also change to local-plugin to make sure it doesn't crash when localStorage doesn't exist (and print a nice warning).

mathieudutour commented 4 years ago

You really need to write somewhere that the file where the model is needs to be called model, I spent so much time trying to understand why everything was undefined. Maybe instead of being undefined until the babel plugin change them, watch and dispatch could be functions that throws with a nice error message

mathieudutour commented 4 years ago

I just now realised that it's all based on Proxy. As far as I know, babel can't polyfill Proxies. It's also only unavailable to more than ~8% of global users (which is a lot) 😕 https://caniuse.com/#search=Proxy

coffee-cup commented 4 years ago

Thanks for the great feedback @mathieudutour! Very good point about mentioning that the model file needs to be called model.

Maybe instead of being undefined until the babel plugin change them, watch and dispatch could be functions that throws with a nice error message

We have noted that using state, watch, and dispatch incorrectly could result in a terrible error message (https://github.com/prodo-ai/prodo/issues/34). We have an experiemental ESLint plugin that will error when using state outside of an action or component, but throwing a nice error message when using watch or dispatch would be really helpful.

I just now realised that it's all based on Proxy. As far as I know, babel can't polyfill Proxies.

This definitly isn't great. We are looking into how we can polyfill proxies for browsers that do not support it. This looks promising, yet still does not polyfill all traps we use.

mathieudutour commented 4 years ago

We have an experiemental ESLint plugin that will error when using state outside of an action or component

I don't think that's enough: if your model is not in model.ts, then it will fail even if used in an action.

So while an eslint plugin would be nice, I think a nice runtime error would be better. Thinking more about it, watch and dispatch aren't enough because watch(state.foo) will fail with can't read foo of undefined because calling the function. So probably need a catch-all proxy that throws all the time.

coffee-cup commented 4 years ago

Yes that's a good point. Relevant issue https://github.com/prodo-ai/prodo/issues/34.