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

Attributes without values breaks the parser #280

Closed
yatesco opened this issue Sep 21, 2022 · 3 comments
Closed

Attributes without values breaks the parser #280

yatesco opened this issue Sep 21, 2022 · 3 comments
Labels
bug Unexpected behavior diffhtml Core API

Comments

@yatesco
Copy link

yatesco commented Sep 21, 2022

Hi,

I've noticed that the parser doesn't like <button readonly> but <button readonly=""> is fine. The error is: htmlElement.setAttribute(lowerName, emptyValue ? EMPTY.STR : value); in the following listing:

  // node_modules/diffhtml/dist/es/node/patch.js
  var { keys } = Object;
  var blocklist = /* @__PURE__ */ new Set();
  var allowlist = /* @__PURE__ */ new Set();
  var setAttribute = (vTree, domNode, name, value) => {
    const isObject = typeof value === "object" && value;
    const isFunction = typeof value === "function";
    const isSymbol = typeof value === "symbol";
    const isEvent = name.indexOf("on") === 0;
    const anyNode = domNode;
    const lowerName = isEvent ? name.toLowerCase() : name;
    const blocklistName = "s-" + vTree.nodeName + "-" + lowerName;
    const htmlElement = domNode;
    if (allowlist.has(blocklistName)) {
      anyNode[lowerName] = value;
    } else if (!blocklist.has(blocklistName)) {
      try {
        anyNode[lowerName] = value;
        allowlist.add(blocklistName);
      } catch {
        blocklist.add(blocklistName);
      }
    }
    if (!isObject && !isFunction && !isSymbol) {
      const emptyValue = value === null || value === void 0 || value === true;
      htmlElement.setAttribute(lowerName, emptyValue ? EMPTY.STR : value); <!-- HERE -->
    } else if (isObject && lowerName === "style") {
      const valueKeys = keys(value);
      for (let i = 0; i < valueKeys.length; i++) {
        htmlElement.style[valueKeys[i]] = value[valueKeys[i]];
      }
    }
  };
...

Unfortunately, this is from the bundled source code so I can't find the exact reference - I hope that's sufficient?

image

NOTE: it tends to fail on a descendant of a node with a valueless attribute, so in this case the child that failed was a grandchild of the element with a valueless attribute.

This bug report is all a bit messy - apologies - I'm on a time critical project ATM - I can return later (next week) with a reproducible project if that would help?

@tbranyen
Copy link
Owner

Sorry :( the old parser was based off node-fast-html-parser, and as such inherited some issues. I have this fixed in the new parser, but still playing whack-a-mole with existing tests. It's getting very close!

@tbranyen tbranyen added diffhtml Core API bug Unexpected behavior labels Sep 23, 2022
@yatesco
Copy link
Author

yatesco commented Sep 23, 2022

No problem at all @tbranyen :-).

One critical bug/feature I use is the fact that it doesn't overwrite input fields if the page has newer data:

  • now: input field = "abc"
  • now+1 request latest from server
  • now+2: input field = "abcdef"
  • now+3 response from server, containing "input field=abc"
  • now+4 merge complete

Crucially, the input field contains "abcdef" not the "abc" from the server. I rely on this, so can we please keep this behaviour (albeit behind a config option maybe?"

@tbranyen
Copy link
Owner

@yatesco I don't anticipate the behavior you're describing changing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior diffhtml Core API
Projects
None yet
Development

No branches or pull requests

2 participants