html-creator icon indicating copy to clipboard operation
html-creator copied to clipboard

BUG?: Ignore undefined values when generating attributes

Open EdgeCaseBerg opened this issue 10 months ago • 2 comments

Ran into this and thought it felt unexpected. If I'm building up an object for attributes in some process like

function makeThing({url, defaultThing = 100, defaultThing2 = undefined}) {
   const attr =  { blablabla, url, defaultThing, defaultThing2 };
   return [{ type: 'div', attributes: attr, content: [...]}]; 
}

Then it throws me off if I pass in something that's undefined because it shouldn't have a value and the resulting HTML generated is like:

<div defaultThing2="undefined" ...>

Does this seem like it would be a good addition to the library as you envisioned it? I know it would handy for me where I have a set of default properties for some elements but in some cases they get additional attributes but otherwise I want to leave them unset and not see them included.

EdgeCaseBerg avatar Apr 18 '24 14:04 EdgeCaseBerg

@Hargne I don't suppose you've had a spare moment to take a look at this?

EdgeCaseBerg avatar May 09 '24 13:05 EdgeCaseBerg

I will have a look asap 😎

Hargne avatar May 09 '24 13:05 Hargne

This is a great addition IMO! 👍🏻 To go even further, why not also exclude null (note that I switched the ternary to a filter to prevent empty strings):

return Object.keys(attributes)
      .filter(
        (key) => attributes[key] !== undefined && attributes[key] !== null
      )
      .map(
        (key) =>
          ` ${key.replace(/([A-Z])/g, (g) => `-${g[0].toLowerCase()}`)}="${
            attributes[key]
          }"`
      )
      .join("");

with a test case:

it("should not include keys when their value is null in attributes", () => {
   const attributeString = Element.applyElementAttributes({
      class: "new",
      dataTest: null,
   });
   expect(attributeString).toEqual(' class="new"');
});

Hargne avatar May 10 '24 12:05 Hargne

Done and pushed that tweak to the branch @Hargne !

EdgeCaseBerg avatar May 12 '24 13:05 EdgeCaseBerg