Add: Auto Sizes for Lazy-loaded Images

[joemcgill]

Summary

Blocked for merge by #934.

This adds a new module that will automatically enhance the sizes attribute of lazy-loaded images to support the new auto syntax.

See: whatwg/html#4654

Fixes: #791.

Relevant technical choices

This adds just the basic functionality needed to implement auto sizes for images created by WP wp_get_attachment_image function or which get responsive attributes added by the wp_filter_content_tags callback. In both cases, auto is prepended to the sizes attribute only if the image already has loading="lazy" applied, which is a requirement to avoid introducing regressions. For more details about compatibility with browser implementations, see this comment.

Checklist

• PR has either [Focus] or Infrastructure label.
• PR has a [Type] label.
• PR has a milestone or the no milestone label.

[joemcgill]

Still need to add some test coverage, but wanted to share an initial implementation for testing/feedback.

[felixarntz]

Thanks @joemcgill, code looks great so far.

One consideration here unrelated to the code itself is how we are going to publish this. A few thoughts:

• We're right now in the last steps of unbundling modules (to be released mid-January). So we probably don't want to add any modules now.
• For that reason I'd recommend to change this PR to go against a new feature branch for this module for now. That way development can continue, and once the repository has shifted to the full standalone plugins monorepo (likely early February) it can be merged into trunk.
• Separately, we may want to already kick off the plugin review process for this? Even if the code as of now isn't necessarily final, it's probably solid enough to submit for review. You could use another experimental or local branch based on this where you add this new module to plugins.json. With that, you can then run npm run build-plugins and get the plugin in a version ready to submit to .org for review.
• The only other thing needed for .org submission is a readme.txt (which we'll need later anyway). Could you maybe add one to this PR? You can probably copy one of the other standalone plugins' readme files and adjust just the few necessary bits.

[joemcgill]

Thanks @felixarntz. Great questions about how we would publish this. This is already essentially the feature branch for this effort, so happy to just keep iterating here and/or submitting additional PRs into this branch. I'll address the rest of your suggestions over the coming week.

The bigger question you raise is how we make modules like this available for people to test before they're published to the wp.org repository once the unbundling effort is complete, given that size of the review backlog at the moment.

I wonder if we can keep some modules in an experimental state where they can be activated from the main performance lab plugin prior to being either promoted to a standalone plugin or as a direct implementation into Core. Right now, you can install the plugin from this repo and checkout this feature branch to test the functionality, but that wont work for modules added after the unbundling effort is complete.

Otherwise, I think we need to update the release script so that zips of pre-release plugins can be downloaded from the https://github.com/WordPress/performance/releases page so we're not completely dependent on the WP.org repo for distributing standalone plugins from this mono-repo.

[felixarntz]

@joemcgill

This is already essentially the feature branch for this effort, so happy to just keep iterating here and/or submitting additional PRs into this branch. I'll address the rest of your suggestions over the coming week.

Happy for you to keep working in here, though the main reason I'd suggest a feature branch is to avoid accidental merge into trunk. As in: Even once this has 2 approvals, we need to determine the best path forward before "just merging". Alternatively, one of us can just keep a "requesting changes" on this to avoid merging "by accident".

I wonder if we can keep some modules in an experimental state where they can be activated from the main performance lab plugin prior to being either promoted to a standalone plugin or as a direct implementation into Core. Right now, you can install the plugin from this repo and checkout this feature branch to test the functionality, but that wont work for modules added after the unbundling effort is complete.

Those are good ideas. We could always consider keeping experimental modules in Performance Lab in the beginning, and once they're approved on .org, we'd phase them out. In that case, maybe the migration logic that @10upsimon and @mukeshpanchal27 have been working on should remain in the plugin even indefinitely, as it would continue to be useful in the future, not only this initial migration push as previously assumed.

[joemcgill]

Even once this has 2 approvals, we need to determine the best path forward before "just merging". Alternatively, one of us can just keep a "requesting changes" on this to avoid merging "by accident".

For the time being, I've converted this to a Draft PR to avoid accidental merge.

I think the conversation about how to handle new modules once we've completed the transition to standalone plugins #656 is worth it's own conversation. I'll open a new issue for us to discuss. I am also going to try to spend some time seeing how difficult it would be to post standalone releases to this repo, since I think that would be useful regardless of whichever future strategy we choose.

[joemcgill]

@felixarntz I forgot that the branch protections on feature branches would block me from continuing to update this branch directly, so I've opened #913 to address the code review feedback and to add initial unit tests.

[joemcgill]

@felixarntz I've opened a new PR against this feature branch that I believe prepares this to be submitted to the the plugin repo for review. I've build the plugin locally by modifying the plugins.json file and have it successfully running on my personal site.

I'm happy to submit this to the Plugin Team for review, but want to make sure that the repo they create will be accessible using the same credentials as the other plugins built from this repo and would appreciate your guidance.

[mukeshpanchal27]

@joemcgill @swissspidy, this PR is ready for merge. The test suite will be handled in #972. WDYT?

[swissspidy]

Good to get the ball rolling 👍

[joemcgill]

I'm happy to have you merge this into the feature/modules-to-plugins branch ahead of the changes being proposed in #972, I think I'm just a bit confused about the sequencing of these changes, but I'm sure it's just me being dense.

[mukeshpanchal27]

Approve again for merge.