jorgebucaran / hyperapp

1kB-ish JavaScript framework for building hypertext applications
MIT License
19.08k stars 780 forks source link

Issue with actions re-using the state #418

Closed Mytrill closed 7 years ago

Mytrill commented 7 years ago

The following unit test fails due to infinite recursion:


test("Recursive state", done => {
  app({
    view: state =>
      h(
        "div",
        {
          oncreate() {
            console.log(document.body.innerHTML)
            expect(document.body.innerHTML).toBe(`<div>{"child":{}}</div>`)
            done()
          }
        },
        JSON.stringify(state)
      ),
    state: {},
    actions: {
      recurse(state) {
        return { child: state }
      }
    }
  }).recurse()
})
Uncaught TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at Object.view (bundle.js:283)
    at render (bundle.js:47)

This is because the update assigns { child: state } to state making it reference itself.

Is it meant to work like this?

jorgebucaran commented 7 years ago

@Mytrill What were you expecting to happen and why are you doing this?

The following unit test fails due to infinite recursion...

Is the infinite recursion caused by Hyperapp or by JSON.stringify(state)? The issue is caused by JSON.stringify because your action is making the state reference itself.

If you are deliberately creating a circular dependency, you can copy the state inside the action:

app({
  actions: {
    recurse(state) {
      return { child: Object.assign({}, state) }
    }
  }
})

...or be careful when using JSON.stringify.

Mytrill commented 7 years ago

Hi Jorge. My use case is: I am implementing a UI where I can recursively split the view in 2. So my state is a tree, and I have a split() action that takes a path in the tree (dot separated string) and insert a node at the given path. The unit test you see here is the equivalent of calling this split() action on the root node.

I already changed my code to use Object.assign({}, state) before opening this issue, but I wanted to know if this was intended behavior (users do the assign in this case, saves perf and bytes to HA) or a bug (HA should always copy state before assigning).

It took me some times to debug this, if there ever is a section "common issues" in the doc, this should be there!

Thanks

Thanks for the answer!