šŸ› Bug with two-way databinding

This issue has been tracked since 2022-11-29.

I was trying to use data binding in arrow-js and ran into a state and render out of sync issue. If you enter a value in the first input, and then enter a value in the second, then the value in the first input does not change. The results are in the screenshot below.

2

atellmer wrote this answer on 2022-11-29
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>arrow-js</title>
</head>
<body>
  <div id="app"></div>
  <script type='module'>
    import { reactive, html } from 'https://unpkg.com/@arrow-js/[email protected]/dist/index.min.mjs';

    const data = reactive({
      text: 'hello',
    });

    html`
      <div>${() => data.text}</div>
      <input value="${() => data.text}" @input="${e => (data.text = e.target.value)}">
      <input value="${() => data.text}" @input="${e => (data.text = e.target.value)}">
    `(document.getElementById('app'))
  </script>
</body>
</html>
justin-schroeder wrote this answer on 2022-11-29

yeah interesting. Attribute binding in arrow uses setAttribute — inputs are weird in that they have their own state with a value content attribute and a value IDL property. AFAIK the IDL property is the one that is actually displayed — I wonder if we should have a few special cases where we detect if the attribute being set has an matching own property (hasOwn) and set it accordingly.

atellmer wrote this answer on 2022-11-30

I think in case of input and maybe textarea you need to patch that element directly by setting their internal property.
At the same time, ignore the setting through the setAttribute property, because in this case there is a small security problem (we can see the password in the attribute through the element inspector for inputs with the password type)

atellmer wrote this answer on 2022-11-30

I would suggest such a solution, which, in addition to setting value directly to the element, also includes the correct processing of boolean attributes when they have a false value, but they are still put down, which affects, for example, html5 validation through the required attribute.

type AttributeValue = string | number | boolean;

function patchProperties(element: Element, attrName: string, attrValue: AttributeValue): boolean {
  const tagName = element.tagName.toLowerCase();
  const fn = specialCasesMap[tagName];
  let stop = fn ? fn(element, attrName, attrValue) : false;

  // check IDL props
  if (Object.getPrototypeOf(element).hasOwnProperty(attrName)) {
    element[attrName] = attrValue;
  }

  // blocking the setting of all boolean attributes, except for data-attributes or other custom attributes
  if (!stop && typeof attrValue === 'boolean' && !attrName.includes('-')) {
    stop = true;
  }

  // we give out the blocking flag for setting this attribute through setAttribute
  return stop;
}

const specialCasesMap: Record<string, typeof patchProperties> = {
  input: (element: HTMLInputElement, attrName: string, attrValue: AttributeValue) => {
    if (attrName === 'value' && typeof attrValue === 'boolean') {
      // checkbox case
      element.checked = attrValue;
    } else if (attrName === 'autoFocus') {
      // autofocus case
      element.autofocus = Boolean(attrValue);
    }

    return false;
  },
  textarea: (element: HTMLTextAreaElement, attrName: string, attrValue: AttributeValue) => {
    if (attrName === 'value') {
      // redirect value to innerHTML
      element.innerHTML = String(attrValue);

      return true;
    }

    return false;
  },
};
justin-schroeder wrote this answer on 2022-11-30

Thanks for the well articulated thoughts and comprehensive code example. Overall its a good direction, a few thoughts on what this implementation should look like (in my opinion):

  1. We need a size efficient way to determine if we need to set the IDL or not. A tagname map of inputs would be too large for a 2kb library. I think we can do this by checking against 3 interfaces: HTMLInputElement, HTMLSelectElement, HTMLTextareaElement. All of those have a value IDL that can be set. I dont think we should set the actual value attribute of any of these. This would fall in line with how Vue does it, but would be in contrast to React.
  2. false removing an attribute is a personal preference of mine that I think leads to much more concise and readable code, despite being a slight deviation from . Vue 2 used to do attributes this way and it switching to false being a valid value, its been a minor pain (in my opinion) and the number of use cases where false is a desired attribute is rare, and in those cases providing the string false is acceptable. So I don’t currently plan to change this part of the implementation — opinionated as it may be.

And with that said — this is now published/fixed in alpha.5 šŸŽ‰

More Details About Repo
Owner Name justin-schroeder
Repo Name arrow-js
Full Name justin-schroeder/arrow-js
Language TypeScript
Created Date 2022-11-08
Updated Date 2023-03-28
Star Count 1240
Watcher Count 21
Fork Count 22
Issue Count 7

YOU MAY BE INTERESTED

Issue Title Created Date Updated Date