preact ignores/rewrites <img> dimensions?
I have a React-managed page with some 175+ images, and after switching from react to preact-compat (using the webpack alias solution) all my images come out as <img src="..." width="0" height="0"> in the DOM, despite having defined dimensions in the JSX source code (e.g. <img src="..." width="100px" height="50px"/> etc) - is there any reason why it does this? Because that seems like a pretty crazy bug.
Pomax
All 28 comments
Code to reproduce is as follows. Note that the JSX has explicit width and height properties for <img> components, which disappear when using preact-compat as react alias.
test.jsx
var React = require("react");
var ReactDOM = require("react-dom");
var Test = React.createClass({
render() {
return <img src='invalid' width="400px" height="400px"/>;
}
});
ReactDOM.render(<Test/>, document.querySelector("#app"));
index.html
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>test</title>
</head>
<body>
<h1>The following box should have an img tag with width and height 100px</h1>
<div id="app"></div>
<p>It doesn't, preact or preact-compat are stripping the JSX props for some reason.</p>
<script src="app.js"></script>
</body>
</html>
webpack config
var webpack = require('webpack');
// Bundle output
var output = {
path: __dirname,
filename: 'app.js',
};
// Necessary webpack loaders for converting our content:
var webpackLoaders = [
'babel-loader',
];
var resolve = {
alias: {
react: "preact-compat",
"react-dom": "preact-compat"
}
};
// And the final config that webpack will read in.
module.exports = {
entry: "./test.jsx",
output: output,
resolve: resolve,
module: {
loaders: [
{
test: /.jsx?$/,
loaders: webpackLoaders
}
]
}
};
package.json
{
"name": "preact",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "webpack"
},
"keywords": [],
"author": "",
"license": "ISC",
"devDependencies": {
"babel-core": "^6.24.0",
"babel-loader": "^6.4.1",
"babel-preset-env": "^1.3.2",
"babel-preset-es2015": "^6.24.0",
"babel-preset-react": "^6.23.0",
"preact": "^3.0.0",
"preact-compat": "^3.0.0",
"webpack": "^2.3.2"
},
"babel": {
"presets": [
"es2015",
"react"
]
}
}
Did a quick test with "real" preact rather than preact-compat, but the same thing happens. Loading the compiled app in the browser shows <img src="..." width="0" height="0">
Pomax
on 2 Apr 2017
@Pomax just don't use pixels there. What happens there is Preact doing img.width = '400px', but width and height of the images are number properties, so conversion to number happens. Number('400px') is NaN, so browser makes it 0.
I don't think Preact itself should fix anything here, but could make sense to fix this in preact-compat (then this issue should be opened in https://github.com/developit/preact-compat repo).
@developit What do you think?
NekR
on 2 Apr 2017
Why are they being converted at all, though? There is no reason to run them through Number(...), just preserve them as whatever the JSX had put in, and write that back out. Is there a reason to "examine" these values instead of just straight-up sending them on as-is?
This feels very much like a case where "too much" is being done - whatever is in those two properties should just end up in the DOM, no matter what it was: even if it was something really silly like width="monkey", preserve it, and offload the responsibility of "does that actually pass spec-validation according to the HTML parser" to the browser. After all, as far as JSX cares it's "just another object property", there's nothing special about them. And someone might be using a client-side script that actually looks for those property values. Unless there's a really good motivation for rewriting them (e.g. "preact becomes a security exploit if we don't"), this is basically still a bug in terms of the real world HTML, especially for mixed content pages that might have a react app but also run other client-side code.
Pomax
on 2 Apr 2017
Okay, step-by-step.
Everything is HTML is a string, that's true. But in JS and DOM it is not. Try running typeof (new Image()).width in console, it will show 'number'.
These properties are not numbers, they are strings.
Properties are number and attributes are strings. Preact creates DOM elements and assigns values to their props (if prop exists in element + few exceptions).
They've always been strings, because they have to contain a unit indicator as per the HTML spec.
Unit indicators in such HTML attributes are artifacts from older versions of HTML, HTML5 however defined only pixels: https://html.spec.whatwg.org/multipage/embedded-content.html#attr-dim-width. You absolutely can use 400px value in pure HTML, this is how browsers keep compatibility.
Even if you leave the unit off, that's still a string, so changing the parsing rules here seems by far the better solution.
You live the unit type off, that's still string, but valid number representation so it could be converted to a number by the browser when assigned to Image's width property.
This feels very much like a case where React does the right thing, Preact does the wrong thing, and this is a bug, not a feature.
It might be a bug per expectations, but that's certainly isn't a bug per how Preact works. After all, you don't pick Preact because it's 100% compatible with HTML and performs all the checks as React does, but it's exactly the opposite why you choose it.
As per preact-compat, I agree, it may need to support this features. That's why I suggest to open an issue about it there.
NekR
on 2 Apr 2017
I think you replied to whatever github sent you as initial notification email; I rewrote my reply after initial post to make more sense.
Pomax
on 2 Apr 2017
@Pomax Unfortunately I'm not going to rewrite my answer.
NekR
on 2 Apr 2017
That is unfortunate indeed, because my rewrite already agrees with many of the points you make - it's more about "JSX doesn't really care, if a property has some value, rewriting it is more work than copying it over and leaving it to the user to have specified a legal value". Especially for apps that need to work within the context of a larger page.
Especially given "After all, you don't pick Preact because it's 100% compatible with HTML and performs all the checks as React does": fully agreed. It seems weird that it does any checks at all in this case, it shouldn't need the code for validation, which would reduce preact's footprint even further.
Pomax
on 2 Apr 2017
There is no special meaning for width/height of images in Preact. It's passed as is to DOM property, as you suggest. Browser converts that value to number though and Number() was just an example to make it clear what happens.
NekR
on 2 Apr 2017
Then maybe I don't understand something here, because if I write HTML with width="200em" in the source (taking preact out of the equasion) then even if the browser interprets that as pixel integers it will preserve that "200em" string in the actual DOM node when I query it. It doesn't rewrite it. So someone's doing rewriting, and it looks very much like it's not the browser? (it might be, but it looks like it's not).
A such I would expect using "200em" in the JSX property to lead to a DOM node with that value still there, not turned into the string "0".
Pomax
on 2 Apr 2017
and leaving it to the user to have specified a legal value
Right, please specify the right value, which is <img src='invalid' width="400" height="400" />. I don't really know how to explain this to you in another way.
Again, attributes and properties are different things. What you see in DevTools are tags with attributes. Actual HTMLImageElement DOM object's width property is of type number.
NekR
on 2 Apr 2017
@Pomax why not just use a style attr?
leeoniya
on 2 Apr 2017
@leeoniya that's too easy :-) Also @Pomax says he needs pixels, so why not just omit px postfix? 馃槈
NekR
on 2 Apr 2017
@leeoniya that incurs a reflow after DOM parse for each image, which you don't really notice for a handful of images, but I have 83 images over a very long article (https://pomax.github.io/bezierinfo) and that gets janky if you load the doc from an anchor low in the document.
I already changed my code to omit the px but that doesn't mean I don't want to understand why this happens, as it breaks expectation without any kind of console warning (either during build, or in the browser). Silently rewriting data generally needs a very good reason, and I don't see it mentioned in the docs or reported during build/at runtime.
If nothing is doing an actual Number() cast, then what exactly is causing this to turn a string into the number zero? (Both preact-compat and preact do this on the same JSX input, where as react with that same JSX doesn't, and that is two-eyebrow-raisingly weird)
Pomax
on 2 Apr 2017
dimension attrs must not have units [1]. so i'm not sure what all the fuss is about.
[1] https://www.w3.org/TR/html5/embedded-content-0.html#dimension-attributes
leeoniya
on 2 Apr 2017
@leeoniya that incurs a reflow after DOM parse, which with 183 images over a very long article getting janky.
DOM isn't being parsed, that's true only for HTML markup. Do you do Server Side Rendering?
If nothing is doing an actual Number() cast, then what exactly is causing this to turn a string into the number zero?
That's done by your browser.
(Both preact-compat and preact do this on the same JSX input, where as react doesn't, and that is two-eyebrow-raisingly weird)
React obviously has a special condition to what where it assigns an attribute instead of a property in that case.
NekR
on 2 Apr 2017
@leeoniya I explained that already. The Topic Starter seems to not just get the different between DOM/HTML, attributes/properties and how JSX works.
I'm really running out of patience because we are figuring out the same thing again and again here, mostly explaining basics about the DOM and HTML. This place isn't the place about teaching web dev.
NekR
on 2 Apr 2017
Fine, fine, look: the last thing I want to do is piss you off, it's a weekend and that's unreasonable. I actually understand all those differences and clearly aren't explaining myself in a way that works for you, so: thank you for the advice to remove the px unit, that absolutely makes things work outside of any "explaining why", and using Preact rather than React has shaved 100kb off the deployed app so far while I'm trying to optimize this thing for people on a bad internet connection, or extremely expensive bandwidth, and preact is helping a lot in addressing their needs.
Apologies for taking up your time.
Pomax
on 2 Apr 2017
@Pomax
has shaved 100kb off the deployed app so far while I'm trying to optimize this thing for people on a bad internet connection, or extremely expensive bandwidth
meanwhile...
I have a React-managed page with some 175+ images
you saved like 1-3 images worth of bandwidth. sounds like you're optimizing in the wrong places. not that i advocate using slow/bloated React
/hijacking
leeoniya
on 2 Apr 2017
@leeoniya executing and parsing additional 100kb of JavaScript is kind of a big deal too. Yeah, bandwidth for images is much bigger, but they don't even start to load while your JS is being parsed/executed. So that's very important too.
_P.S. I often find people using "we load huge images anyway" as an excuse to have MBs of JS._
NekR
on 2 Apr 2017
yeah that 100kb is actually purely in the JS footprint (~700KB for React to about ~600KB with Preact), not the images themselves: those make up an out-of-app 500KB in .svg resources when sent with gzip enabled, so saving 100kb off the JS is super nice, but even on the total transfer it has quite an impact . Of course, the insane number of http requests those images incur on an empty cache can also do with some optimization, but bundling them into the app itself when the images can change independent of it seemed a bad idea.
The big issue now is figuring out how to split out "this is actually mostly regular HTML content that doesn't even need to be JSX with a sprinling of really-is-UI-components" so that it doesn't take up those app bundle bytes anymore either. React's server side rendering was kind of an all-or-nothing deal, where you either render to incredibly verbose HTML source code that the app can hook back into, but then you still have the original app size and a very large initial html payload, or you serialize to very clean HTML but then the app can't hook the interactive parts back in. But that's a puzzle for another day.
Pomax
on 2 Apr 2017
@NekR i know, i have a lib faster than preact in this space [1], though i would never promote using js server-side to generate html, even though domvm has very fast SSR/isomorphism (~50x React's).
i detest jsx and mountains of build dependencies that React & friends promote.
@Pomax
Of course the insane number of http requests those images incur
HTTP2, hopefully everywhere soon. Until then (and even afterwards), just use scroll position to load them on demand.
leeoniya
on 2 Apr 2017
@leeoniya Yeah, greatly looking forward to HTTP2 making it into browers and servers universally, a lot of sites will benefit from that.
Pomax
on 2 Apr 2017
@Pomax - It sounds like you're still not sure what's going on. I'll try to see if I can explain:
Internally, what happens when Preact renders the <img src='invalid' width="400px" height="400px"/> in your JSX is roughly this:
- The JSX code above is turned by Babel into
h('img', { src: 'invalid', width: '400px', height: '400px' }) - The result of the
h(...)function call is a data structure that looks like this:
{
nodeName: 'img',
attributes: {
src: 'invalid',
width: '400px',
height: '400px',
},
children: [],
}
- Internally, Preact then takes the data structure from step (2) (
vdomElementbelow) and creates an actual DOM element:
var node = document.createElement(vdomElement.nodeName);
// ...
for (var key in vdomElement.attributes) {
// Does `node` have a property matching the prop name?
if (key in node) {
// Yes, set the property.
node[key] = vdomElement.attributes[key];
} else {
// No, use `setAttribute`
node.setAttribute(key, vdomElement.attributes[key]);
}
}
// ...
parentElement.appendChild(node);
- In the case when
vdomElement.nodeNameisimg,nodeis anHTMLImageElement, which _does_ have properties calledwidthandheightso what ends up getting executed is:
var node = document.createElement('img');
node.src = 'invalid';
node.width = '400px';
node.height = '400px';
Try the code from step (4) in your browser console and then print out the values of node.width and node.height.
Now on the other hand, try this:
// This is what happens when the browser parses the HTML string '<img width="400px" height="400px">'
var node = document.createElement('img');
node.setAttribute('width', '400px');
node.setAttribute('height', '400px');
console.log(node.width, node.height);
You'll see different results.
Does this make things clearer?
robertknight
on 2 Apr 2017
I'll add that you've now encountered the downside of Preact compared to React. Whereas React has a ton of checks and debugging messages in the codebase to warn you when you do something like this, Preact has very few. Consequently, it is harder to "muddle through" if your mental model of what the library is doing is inaccurate.
robertknight
on 2 Apr 2017
There is an existing related issue for the difference between React and Preact here: https://github.com/developit/preact/issues/492
robertknight
on 2 Apr 2017
@robertknight it does, thanks.
Pomax
on 2 Apr 2017
Sorry for missing this discussion - thanks @robertknight for pointing to the previous issue. Preact can't afford to bundle large whitelists/mappings for DOM edge cases, so right now it uses an in check to determine if a property should be used, otherwise falling back to attributes. That behaviour handles a lot of cases, but is imperfect when DOM properties are mismatched to attributes of the same name. Technically what Preact is doing is correct here, but it does differ from React and cause a bit of confusion - JSX properties look like attributes, so when they get used to set DOM properties that can be a little odd (even though it's the most common case).
FWIW, there's a PR in that, if landed, would fix this: https://github.com/developit/preact/pull/511
As for a modification to preact-compat (if we go that route), a "hack" here that forces attributes instead of properties is to change the casing of the prop name slightly, so that 'width' in img is false. It could be done as a vnode transform in compat, but here's the literal version:
<img width="10em" /> // tries to set img.width = "10em" (fails)
<img Width="10em" /> // img.setAttribute("Width", "10em") (works)
developit
on 2 Apr 2017
Related issues
rajaraodv
路
3Comments
KnisterPeter
路
3Comments
philipwalton
路
3Comments
adriaanwm
路
3Comments
Zashy
路
3Comments
Most helpful comment
dimension attrs must not have units [1]. so i'm not sure what all the fuss is about.
[1] https://www.w3.org/TR/html5/embedded-content-0.html#dimension-attributes