preact: simple property assignment fails to set img width

I’m facing some incompatibility when replacing react with preact. It seems the way preact sets attributes (https://github.com/developit/preact/blob/master/src/dom/index.js#L68 - simply by setting a property) is not safe.

My use case is a simple img component that receives width and breaks with the value of 100% because of the the following:

const img = document.createElement('img');
img.width = '100%';
console.log(img.width); // 0

And what I think should be done (this is what react does)

const img = document.createElement('img');
img.setAttribute('width', '100%');
console.log(img.width); // expected value (automagically defined by the browser)

Unfortunately, changing setProperty to

function setProperty(node, name, value) {
  try {
    node.setAttribute(name, value);
  } catch (e) { }
}

isn’t enough… I think we might have to do some checks before knowing which kind of property setting we should do (simple assignment or setAttribute)… Any thoughts?

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 23 (13 by maintainers)

Most upvoted comments

It’s an open bug, typically open bugs represent broken promises.

developit on Aug 21, 2017

@enapupe I’m open to the test idea - it was also proposed in #242.

Regarding the String suggestion - the reason this is useful is because Preact cannot support property whitelists given the filesize constraints we have placed on the library. Instead, we need to look for other ways to implement conditions - if the String type of a value can be used to fix this bug without causing a performance degradation for other String props, that’s probably what will end up being used since it’s only a couple of bytes (and thus much smaller than a whitelist).

It’s also worth noting that I think this issue would apply more broadly to other DOM properties. height is the obvious one, but there are others - for example list and type are already blacklisted for similar reasons. Maybe our solution to this issue will enable us to remove those specific blacklists and reduce the bundle size 😃

developit on Jan 11, 2017

@enapupe it’s a little specific, but at least that lets us track the error. Much appreciated 😃

developit on Jan 10, 2017