Ideally, this assertion could be rewritten as the following:

[null, undefined, false].includes(tagDefinition.attributes[attributeName])

It has better readability, but due to lint constraints that was not possible.

What about tagDefinition.attributes[attributeName] === '' || tagDefinition.attributes[attributeName]?

That should be equal to your code

This would introduce a bug in the plugin due to the nature of truthy values, which excludes false, 0, "", null, undefined and NaN values.

As you can see, 0 as value might be needed.

but in your typings you didn't allow numbers - should we add them too?

that's actually a good point! I'm not sure about the broad use case of it, but if we're going to proceed in this way, then we can simply refactor it to:

  const attributes = Object.keys(tagDefinition.attributes || {})
    .filter(function (attributeName) {
      return tagDefinition.attributes[attributeName];
    })

This would satisfy my use case, although I would appreciate your feedback regarding it, as you might have a better sense of the community usage.

e.g.:

/**
 * @param {undefined | null | string | number | boolean} value
 */
function hasValue(value) {
  if (value === undefined || value === null || value === false) {
    return false;
 }
 return true;
}

Or we just keep your initial proposal :)

The idea to have this hasValue is interesting, but if the usage is restricted to this single case, have the assessment inline provides better understand about the code, so I would go for that suggestion of yours:

tagDefinition.attributes[attributeName] === '' || tagDefinition.attributes[attributeName]

This fits nicely

Okay cool so we have two valid options I guess:

1. The one you already committed (which also allows numbers)
2. The tagDefinition.attributes[attributeName] === '' || tagDefinition.attributes[attributeName] which does not allow numbers

I am fine with both ways :)

I have applied the suggestion you made, as you have a better understanding about its usage.

Thank you for the nice brainstorming

Also fixed the JSDoc definitions for object entry

Can we please keepattributeName and add undefined & null?

Of course, do you want me to restore both changes on the JSDoc?

Yes and please also for typings.d.ts :)

The change on typingins.d.ts are necessary to reflect the change

You are right :) sorry I didn't see typings.d.ts - here it should be the same as in typings.d.ts:

please keep the old test and just add 2 more - 1 for null 1 for undefined..

that way we get better error messages if 1 test fails and to guarantee that the old tests are not modified and still work