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

Value Unwrapping and Wrapping. #1

Open
littlehaker opened this issue Sep 23, 2019 · 17 comments
Open

Value Unwrapping and Wrapping. #1

littlehaker opened this issue Sep 23, 2019 · 17 comments

Comments

@littlehaker
Copy link

In Vue 3.0 function based api, atomic type value and state are handled different. When nest a value in a state, the value is unwrapped.

const count = value(0)
const obj = state({
  count
})

console.log(obj.count) // 0

obj.count++
console.log(obj.count) // 1
console.log(count.value) // 1

But in Bistate, there's just state. So when nested, it's not unwrapped.

const count = useBistate({ value: 0 })
const obj = state({
  count
})

console.log(obj.count) // expected 0 but got { value: 0 }

Besides, when passing a value to another component, it should be wrapped in a field such as value. Can Bistate automatically wrap an atomic type to a value, so we can make the state more clear and readable?

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo: { value: 'some value' }
  })  // expect { foo: 'some value' } but must be { foo: { value: 'some value' } }
  return <Input value={state.foo} />
}
@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 23, 2019

I have no idea how to update state.foo from the child component to the parent component, since the child component just receives a string.

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo:  'some value' 
  }) 

  // state.foo is a string, if we wrap state.foo inside Input, how to notify App?
  return <Input value={state.foo} />
}

I suggest computed state via useMemo.

function Input({ text }) {
  const onChange = useMutate(e => {
    text.value = e.target.value;
  })
  return <input value={text.value} onChange={onChange} />
}

function App() {
  const foo = useBistate({ value: 'some value' } );
  const bar = useBistate({ value: 'other value'});

  // state is a plain javascript object
  // do not mutate it
  // treat it as read only
  // useMemo guarantee the state always fresh when deps list changed
  const state = useMemo(() => {
    return { 
      foo: foo.value, 
      bar: bar.value 
    }
  }, [foo, bar])

  useEffect(() => {
    // use state to do what you want
  }, [])

  return <Input value={foo} />
}

@littlehaker
Copy link
Author

How about using a special proxy key to wrap the atomic value?

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo: 'some value' 
  }) 

  // state.$foo for { value: 'some value' } and state.foo for 'some value'
  // when 'some value' changes in `Input` component, update state.foo
  return <Input value={state.$foo} />
}

@Lucifier129
Copy link
Owner

How about using a special proxy key to wrap the atomic value?

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo: 'some value' 
  }) 

  // state.$foo for { value: 'some value' } and state.foo for 'some value'
  // when 'some value' changes in `Input` component, update state.foo
  return <Input value={state.$foo} />
}

Maybe it can make sense, but we must find a good way to support this pattern.

Instead of adding extra property key, binding(bistate, key) function should be better, just like vue 3.0 reactivity api.

@Lucifier129
Copy link
Owner

Let's think about it.

PR is welcome!!

@Lucifier129
Copy link
Owner

I am working in the process for binding features now~

@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 24, 2019

@littlehaker

I want to know your feeling about the binding API below, it's ok to you or not?

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo:  'some value'
  }) 

  return <Input value={binding(state, 'foo')} />

  // or
  const foo = binding(state, 'foo')

  return <Input value={foo} />
}

@littlehaker
Copy link
Author

It's OK for this case.

But maybe we can do a more common design, use a Get/Set pair to specify the binding rule, and make some shorthands.

const foo = state.$foo
// is short for
const foo = binding(state, 'foo')
// is short for
const foo = binding(state, {
  get: s => s.foo,
  set: (val, s) => s.foo = val,
})

@Lucifier129
Copy link
Owner

@littlehaker It's good, it looks even better if we use hooks

const foo = useBinding({
  get: () => state.foo,
  set: (value) => state.foo = value
})

@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 24, 2019

I think the API names should be changed when the features become more general

const foo = useComputed({
  get: () => state.foo,
  set: value => state.foo = value
})

// or
const foo = useComputed(
  () => state.foo, 
  value => state.foo = value
)

// and then
const useBinding = (state, key) => useComputed({
  () => state[key],
  value => state[key] = value
})

// we got
const foo = useBinding(state, 'foo')

@littlehaker
Copy link
Author

👍 Good for that.

@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 25, 2019

@littlehaker Sorry for changing API frequently. But I found we maybe go too far away from our original purpose.

useComputed accepts get/set function in function-component, it is too powerful to keep data consistent.

It's hard to prevent users write code like below. Tracing multiple bistate in computed causes updating state indirectly, and less reasonable for now. We need more research to introduce them.

const foo = useComputed({
  get: () => {
     return [stateA.a, state.B.b]
  },
  set: value => {
    stateA.a = value[0];
    stateB.a = value[1]
  }
})

I propose a new pattern for binding API, non-selector-based, no hacky property like $foo.

function Input({ value }) {
  const onChange = useMutate(e => {
    value.value = e.target.value;
  })
  return <input value={value.value} onChange={onChange} />
}

function App() {
  const state = useBistate({
    foo:  'some value'
  }) 

  return <Input value={binding(state).foo} />

  // or
  const { foo } = binding(state)

  // or
  const { foo } = useBinding(state)

  return <Input value={foo} />
}

Looking forward to your feedback!

@littlehaker
Copy link
Author

@Lucifier129

The idea of Get/Set comes from use-profunctor-state.

Your newly revised API is good for objects, but its power is limited for data transform. For example:

const fahrenheit = useBistate({ value: 10 });

const celsius = useComputed({
  get: () => fToC(fahrenheit.value),
  set: (val) => fahrenheit.value = cToF(val),
});
// Cannot be done with binding

To avoid tracing multiple bistate while keeping the power, we may still offer a method to provide common Get/Set operation. Like below:

const foo = binding(state).foo
// is short for
const foo = binding(state, {
  get: s => s.foo,
  set: (val, s) => s.foo = val,
})

const celsius = binding(fahrenheit, {
  get: (f) => fToC(f),
  set: (c, f) => f.value = cToF(c),
});

@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 26, 2019

@littlehaker I have tried some implementation, and I figured out that none of them is better than useMemo with getter/setter.

const celsius = useMemo(() => {
  return {
    get value() {
      return fToC(fahrenheit.value)
    },
    set value(value) {
      fahrenheit.value = cToF(value)
    }
  }
}, [fahrenheit.value])

is better than

let celsius = useComputed(
  {
    get: () => ({ value: fToC(fahrenheit.value) }),
    set: ({ value }) => {
      fahrenheit.value = cToF(value)
    }
  },
  [fahrenheit.value]
)

It's easy and natural to handle multiple fields

const celsius = useMemo(() => {
  let computed = {
    get value() {
      return fToC(fahrenheit.value)
    },
    set value(value) {
      fahrenheit.value = cToF(value)
    },
    // add other fileds
    get text() {
      return `${computed.value}°`
    }
  }
  return computed
}, [fahrenheit.value])

@Lucifier129
Copy link
Owner

Lucifier129 commented Sep 26, 2019

There is still some work we can do, such as handling stale closure, we may introduce useComputed and useBinding based on useMemo

let computed = useComputed({
  get value() {
    return fToC(fahrenheit.value)
  },
  set value(value) {
    fahrenheit.value = cToF(value)
  }
}, [fahrenheit.value])

let state = useBistate({ foo: 'abc' })
let { foo } = useBinding(state)

foo.value

@Lucifier129
Copy link
Owner

@littlehaker useComputed and useBinding are implemented in bistate.

You can click the links above to see how to use.

Give me feedback when you have any question.

We will ship these features in version 1.3.0 later.

@littlehaker
Copy link
Author

Hi @Lucifier129. It looks really nice, what a great work!

@Lucifier129
Copy link
Owner

Thx, bistate v1.3.0 has been published to npm, it's available now.

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

No branches or pull requests

2 participants