renatorib / react-powerplug

:electric_plug: Renderless Containers
https://renatorib.github.io/react-powerplug
MIT License
2.68k stars 101 forks source link

fixed Value reset error #181

Closed harrysolovay closed 5 years ago

harrysolovay commented 5 years ago

I was getting the following error when trying to call reset on Value

screen shot 2018-10-17 at 8 32 36 am
TrySound commented 5 years ago

@harrysolovay How do you call reset?

harrysolovay commented 5 years ago

@TrySound I call it by using the State component & passing the resetState render-prop into onClick:

import React from 'react'
import { State } from 'react-powerplug'
import { render } from 'react-dom'

const App = () =>
  <State initial={{ count: 0 }}>
    {({ state, setState, resetState }) => (
      <div>
        count: { state.count }
        <div>
          <button
            onClick={() => setState(({ count }) => ({
              count: count - 1,
            }))}
            children='decrement'
          />
          <button
            onClick={() => setState(({ count }) => ({
              count: count + 1,
            }))}
            children='increment'
          />
          <button
            onClick={ resetState }
            children='reset'
          />
        </div>
      </div>
    )}
  </State>

render(<App />, document.getElementById('root'));
TrySound commented 5 years ago

Well, this is the problem. reset accepts a function, but you pass event object. Just use arrow function there.

  onClick={() => resetState()}
harrysolovay commented 5 years ago

Does that matter though? My understanding is that––in many use cases––users won't want to pass a callback to reset. For the sake of more concise usage, it's beneficial to avoid error-throwing when reset's argument is not a function (or specifically when it's an event)... the passing of event data into the callback doesn't need to create errors (maybe a console.warn instead).

Please let me know how / if the PR can be improved. I understand if you like the constraint and would rather scrap this 👍

Just trying to help out :)

TrySound commented 5 years ago

Well, it's a type error. Type errors should be handled statically or runtime will be too big and slow.

harrysolovay commented 5 years ago

Gotcha 👍

harrysolovay commented 5 years ago

I just thought it would make it easier for users without type systems, who don't know why the shorter syntax isn't working. But I agree, the passing of the event data is unnecessary to begin with.