Set image width/height attributes in HTML

[thewilkybarkid]

↪️ Pull Request

This sets the width and height attributes on HTML <img> elements to help layout in browsers (refs #3738 (comment)).

I had been using posthtml-img-autosize (with the processEmptySize option), but that's incompatible with things like the image transformer (since it runs before transformations).

💻 Examples

See the tests.

🚨 Test instructions

Tests included.

✔️ PR Todo

• Added/updated unit tests for this change
• Filled out test instructions (In case there aren't any unit tests)
• Included links to related issues/PRs

[height]

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

[thewilkybarkid]

The potential layout problem is if someone has something like img { width: 100% } without height: auto (since the image will stretch). This is combination 3 on WICG/intrinsicsize-attribute#16 (comment).

Or are using the width/height to control the size (refs #6783 (comment) and #3738 (comment)).

Feels like the benefits far outweigh this edge case though.

[thewilkybarkid]

As it stands, these are both fixed to be the real size of the image.

[thewilkybarkid]

Would be changed by #7390

[thewilkybarkid]

I'm planning on experimenting with injecting a little CSS to avoid misshapen images (which is hopefully tree-shaken away in most cases). (Too much for now.)

[devongovett]

I think we should only do this if a width or height attribute is not already set on the element?

[thewilkybarkid]

I think we should only do this if a width or height attribute is not already set on the element?

Probably. I left this in for the moment (#6783 (comment)) for feedback (primarily about the CSS issue), but in the longer term it would be better for these to be "fixed" earlier in the pipeline (refs #3738 (comment)).

[mischnic]

We never want to make Parcel "break" your code after bundling, so if it worked correctly before bundling, it should still behave the same afterwards. That is unfortunately not always the case here:

https://codesandbox.io/s/serene-ives-legx7?file=/index.html

So I'm torn between automatically adding them to avoid layout shift and not randomly breaking the projects of unknowing users (especially because this is hard to debug if you don't know what you're looking for. In this case, you need to set the other "free" dimension to auto instead of initial to retain the aspect ratio).

[thewilkybarkid]

So I'm torn between automatically adding them to avoid layout shift and not randomly breaking the projects of unknowing users (especially because this is hard to debug if you don't know what you're looking for. In this case, you need to set the other "free" dimension to auto instead of initial to retain the aspect ratio).

I do wonder about this too; the packager doesn't feel the right place to do it but I'm not sure where is (the HTML transformer's too early?).

I did wonder about injecting some CSS (#6783 (comment)), but that also could be thorny (and I'm not sure how and where it should happen: inline and before any CSS in all HTML files that contain images?).

[thewilkybarkid]

If it helps, this could be split in two: the first part that adds the width/height as metadata on image assets, then a follow-up that uses it to set the width/height in HTML. Since the latter has potential CSS problems, I could release this as a separate plugin. (The former could also be a separate plugin, but doesn't seem unreasonable to make that information generally available for image assets?)

the packager doesn't feel the right place to do it but I'm not sure where is (the HTML transformer's too early?).

Looking at the plugin system again, would it be an optimizer? (It's not 'optimization' as such, but needs to be a late-running transformation.)

[thewilkybarkid]

Looking at the plugin system again, would it be an optimizer? (It's not 'optimization' as such, but needs to be a late-running transformation.)

I had a go at writing an optimizer and managed to get it to work, it just seems a bit tricky to find the asset from the src (which is now the final path, e.g. /some-image.ab12345.jpg).

First attempt looks like:

tree.match({tag: 'img', attrs: {src: true}}, node => {
  let image;

  for (const dependency of dependencies) {
    const asset = bundleGraph.getResolvedAsset(dependency, bundle)

    if (!asset) {
      continue;
    }

    const bundles = bundleGraph.getBundlesWithAsset(asset);

    if (node.attrs['src'] !== bundles[0].name) {
      continue;
    }

    image = asset;
    break;
  }

  // use imageSize on image and update attributes on node

  return node;
});

which works, but is rather ugly. Is there a simple way that I've missed?

Does either of these approaches make sense @devongovett @mischnic? I've happy to update this PR or close it and release something standalone.