Bug with Input, checkedLink and set/onChange/etc

[richard-engineering]

So I have been debugging this for my coworker and I think I discovered a bug with Input.

The situation is that we have two checkboxes. If the first checkbox is ticked, it should set the second checkbox to unticked.

Functional code:

// componentLink = Link.state((parent_obj), 'component_data')
// componentLink.value = {
//   'first_checkbox_key': { 'enabled': [true/false] }
//   'second_checkbox_key': { 'enabled': [true/false] }
// }
const Container = (props) => {
  const { links } = props;  // Directly passing componentLink in props also works
  const { componentLink } = links;
  return
    <div>
      <FirstCheckbox links={links}/>
      <SecondCheckbox links={links} />
    </div>
};

const FirstCheckbox = (props) => {
  const { links } = props;
  const { componentLink } = links;
  const secondEnabledLink = componentLink.at('second_checkbox_key').at('enabled');
  const firstEnabledLink = componentLink.at('first_checkbox_key').at('enabled');
  const change = firstEnabledLink.action((old, event) => {
    const checked = event.target.checked;
    firstEnabledLink.set(checked);
    if (checked) {
      secondEnabledLink.set(false); 
    }
  });
  return <div>
      <input type='checkbox' value={firstEnabledLink.value} onChange={change}/>
  </div>
};

const SecondCheckbox = (props) => {
  const { links } = props;
  const { componentLink } = links;
  const secondLinkEnabled = componentLink.at('second_checkbox_key').at('enabled');
  return <div>
      <Input type='checkbox' checkedLink={secondLinkEnabled}/>
  </div>
};

tags.jsx/Input and CheckedLink doesn't work Replacing the first checkbox with a checkedLink using Input from tags.jsx and an onChange doesn't work.

  const firstEnabledLink = componentLink.at('first_checkbox_key').at('enabled')
   .onChange((enabled) => {
     console.log("I was hit", enabled);
     if (enabled) {
       console.log("I am disabling thingy");
       secondLink.set(false);
       console.log(secondLink.value);  // Remains 'true'
    }
  });
  ...
  <Input type='checkbox' checkedLink={firstEnabledLink}/>

Adding an onChange function the normal way also doesn't work

 const handle_click = (firstLink) => {
   return function (e) {
     const val = e.target.value;
     if (val === 'false') {
       firstLink.value = false; //Also tried .set(false), .update((x)=> false), .action
       console.log(firstLink.value);
     }
   }
 };
 ...
 <Input type='checkbox' checkedLink={firstEnabledLink} onChange={handle_click(firstEnabledLink)}/>

[gaperton]

       purchaseAmountEnabledLink.set(false);
       console.log(purchaseAmountEnabledLink.value);

When you set the link, link.value should not change. As it will change some upper state, and link object will be recreated. Links themselves are immutable.

However, this code behaves not according to the logic you described. Look at the inline comments:

if (!enabled) { <- when new link value is false...
       console.log("I am disabling thingy");
       purchaseAmountEnabledLink.set(false); <- ...you set another link to false. Should it be vice versa?
       console.log(purchaseAmountEnabledLink.value);  // <- that's remains 'true' as links are immutable.
    }

And onChange doesn't work with Input. It expects just valueLink or checkedLink.

[gaperton]

Also, the behavior you described here seems to be close to the radio-buttons. There's an easier way to represent radio group state. In your case, just one boolean flag is enough.

https://github.com/Volicon/NestedLink#radio-groups-and-select-list

Also, there's another way of handling data cross-dependencies. You can apply pipe to the componentLink function, and handle cross-dependencies inside of pipe's argument. In this case, there will be just one atomic update of the state (and one render) on checkbox click.

https://github.com/Volicon/NestedLink#-linkpipe-transform--any--any---link

[richard-engineering]

Ah yes, I copied the wrong code over into the issue, was cleaning up the code to remove unnecessary things (I initially misunderstood what the use case was my coworker was having problems with and thought it was on uncheck, that was copied from older code that equally didn't work). No it's not radio buttons, we actually want to allow multiple selections (there are more than 2 checkboxes on the actual screen, I simplified the code considerably in the issue).

[richard-engineering]

When you set the link, link.value should not change. As it will change some upper state, and link object will be recreated. Links themselves are immutable.

How come this doesn't work then? Is it something with React not propagating a change down then?

  const firstEnabledLink = componentLink.at('first_checkbox_key').at('enabled')
   .onChange((enabled) => {
     console.log("I was hit", enabled);
     if (enabled) {
       console.log("I am disabling thingy");
       secondLink.set(false);
       console.log(secondLink.value);  // Remains 'true'
    }
  });
  ...
  <Input type='checkbox' checkedLink={firstEnabledLink}/>

Btw: thanks for the quick response Edit: Fixed misnamed link variable in example, it was not misnamed when I tested it (blame my editing of the code to make the example clearer)

[gaperton]

Here, when you disable first checkbox, you're disabling the second one instead of enabling it.

if (!enabled) { // <- this condition should be flipped, I guess
       console.log("I am disabling thingy");
       purchaseAmountEnabledLink.set(false);

[gaperton]

Ok, about pipe. You could do it like this. Just do npm update.

function checkboxRules( next, prev ){
    if( next.first.enabled && !prev.first.enabled ){
         next.second = { enabled : false };
    }

   ...
   return next;
}

const smartLink = componentLink.pipe( checkboxRules );

So, all of your rules will be checked and applied in a single place, and you will have one atomic UI update no matter how complex they are. You have to detect what have changed manually, though. Comparing next and prev.

That's just an alternative approach to the problem when you move these rules checking upper. Must result in cleaner code. Which is way easier to reason about.

[richard-engineering]

Ahh, saw your commit a few minutes ago. Is next the old state and prev the new state? I suppose I can console.dir that. I think I understand your example after staring at it, will test and get back to you with any questions.

[gaperton]

next is new state, prev is prev state. Whatever you return substitutes the next state.

[richard-engineering]

Okay I've implemented a test file and with pipe behaves very strangely with Input from 'valuelink/tags', I'll upload it to a project in a bit so you can run it yourself and see.

[richard-engineering]

https://github.com/rhan-mentad/valuelink-debug

(sorry about the cruft, was getting weird path errors trying to port over a smaller webpack config we use so I gave up and used the original boilerplate project as the base) Do npm i followed by npm start, navigate to localhost:3001.

Changes are in client/main.jsx

If you comment out the pipe command, the Input checkedLink functions fine. If the pipe command is in there then you cannot select the second checkbox after it gets deselected by clicking the first checkbox.

[richard-engineering]

Nvm I think I was an idiot, mixed up old and new state in the callback. Everything works great for our use case. Thanks for the help :)

[gaperton]

Okay, just for the case - I'm copying my fix to your example here:

function stateRules( next, prev ){ // first argument is new state, second - the previous one.
  if( next.second_checkbox_key.enabled && !prev.second_checkbox_key.enabled ){
    next.first_checkbox_key = { enabled : false };
  }

  if( next.first_checkbox_key.enabled && !prev.first_checkbox_key.enabled ){
    next.second_checkbox_key = { enabled : false };
  }

  return next;
}

And

const componentLink = Link.state(this, 'container').pipe( stateRules );

[gaperton]

Yes, also:

  getInitialState: function () {
    return {
      container: {
        second_checkbox_key: {
          'enabled': false // Your initial state should be valid according to your rules.
        },
        first_checkbox_key: {
          'enabled': true
        }
      }
    }
  },

It works as you want. :)