I2D: Deprecate "-latest" Binaries

[samouri]

Summary

Context: Following up on an old comment #21135 (comment)

The original intention was that if we create a new version of a component which maintains backward compatibility, we could then simply modify the "-latest" pointer to the new version. Publisher documents could then seamlessly get the new version with no action on their part.

In practice we don't create a new version of a components unless we are explicitly making breaking changes.
This means that bumping the pointer is impossible as it will more than likely break current consumers (as happened in the past). Therefore the -latest binaries have serve no practical purpose and we should remove them.

Deprecation Plan

1. Modify the documentation for -latest binaries on amp.dev to point out that it is deprecated.
2. Any new components created should not allow -latest via validator rules.
3. Add a validator warnings for pages using -latest binaries, recommending publishers pin to a specific extension version.
4. Simplify the configuration in our bundles configuration s.t. they only provide a latestVersion when it isn't the value 0.1
5. Eventually, if usage is low enough, remove the binaries entirely. (cc @honeybadgerdontcare). Less than 1 in 1,000 would be "low enough". removed based on discussion from design review.

Notifications

/cc @ampproject/wg-approvers

[Gregable]

I'd suggest an intermediate step - new components will not allow -latest binaries in the validator. This can't break existing pages since nobody uses the new component yet. It's also trivial to do. It's a policy in code review, not an actual code change.

[Gregable]

For approval, I'd like to see some explicitly recommended metric for "usage is low enough". Given that we'd be breaking existing pages with usage, I would only approve it was extremely low. For example, fewer than 1 in 1,000 documents which include that particular component.

[samouri]

I'd suggest an intermediate step - new components will not allow -latest binaries in the validator. This can't break existing pages since nobody uses the new component yet. It's also trivial to do. It's a policy in code review, not an actual code change.

Added this as another step in the deprecation plan.

For approval, I'd like to see some explicitly recommended metric for "usage is low enough". Given that we'd be breaking existing pages with usage, I would only approve it was extremely low. For example, fewer than 1 in 1,000 documents which include that particular component.

Sure, added. I'm actually ok with keeping it forever, as long as its pinned to its current version.
But removal at some point (if fewer than 1 in 1,000) would be nice.

[Gregable]

First Approval

[kristoferbaxter]

Approved with the removal of step 5.

[samouri]

Summary of design review feedback:

1. Locking the version -latest points to is ok, but we should never stop producing the binaries.
2. For new components we should not generate -latest.

[rsimha]

Approved now that step 5 has been removed.

Edit: Chatted offline with @samouri to clarify step 4 and to see if there is a way to implement it without special-casing 0.1 components. He will update with a potentially simpler approach.

[samouri]

Much of this has been completed in #36889.
All that remains is the validator warning

[westonruter]

By removing the latestVersion we're now left without a reference for which components are the latest stable version. Since most of the 1.0 versions relate to Bento versions which are all experimental, this is causing problems for the AMP Optimizer's AutoExtension transformer (in both Node and PHP). It's also problematic for the AMP plugin for WordPress, which still has it's own extension auto-importer logic.

We need a new canonical source where we can easily read the latest stable version that can be used for auto-extension importing. Or else, we need an easily way to see which extension versions are experimental. (Even experimental extensions can be valid, so the validator spec isn't suitable.)

[samouri]

We need a new canonical source where we can easily read the latest stable version that can be used for auto-extension importing. Or else, we need an easily way to see which extension versions are experimental. (Even experimental extensions can be valid, so the validator spec isn't suitable.)

What is the expectation of AutoImporter:

• Are folks expected to run the optimizer once on an HTML file that may be missing scripts in the head, and it will fill those out. The fixed file is then checked in to version control and this transform is never run again.
• Folks running AutoImporter at serve (or server-boot) time in which case it reruns periodically.

If the former, then the desire for "latest non-experimental" version makes sense! If the latter, then the concept itself seems like a mistake. Major version bumps are expected to have breaking changes which could easily break all documents depending on the importer to function.

[westonruter]

It could be either of those two cases, depending on how the Optimizer is being used in the application.

The AutoExtensions/AutoExtensionImporter transformer does have a configuration option to provide a mapping of component versions to use. So if you want to consistently use the same version (as you should) then that should be supplied. Also, if the transformer is provided with AMP document that already has an component's script present, then the import is skipped. So in that case, it's just needed to supply extensions for components that lack scripts entirely. So in this case the transformer needs to have a “best guess” of which component version to use. Until recently this was the latestVersion, but if it can switch to use the greatest version that is not experimental, then that would work just as well.

In the context of the AMP plugin for WordPress, the auto-extension importing is tied to plugin version. So if a component version is going to bump from 0.1 to 1.0, it will only happen in the context of a plugin version bump. We can test to make sure any version bumps work properly while we QA the release. In that case, if the component version bump causes a problem then the author can manually pin it to an older version. Also, the plugin has at times selectively pinned extension versions to differ from the latestVersion. For example, with amp-carousel are serving 0.2 by default even though latestVersion was 0.1. And then for a time amp-experiment had its latestVersion set erroneously to 1.0 which was still experimental, so we had to pin it manually to 0.1.

So yeah, if we had a standard reference for which component versions were experimental, we'd be able to avoid ever using those versions when we have to automatically import an extension.

[schlessera]

Another use case that has its own set of challenges is that we want to work on automatically replacing slow components (like a popular React slider known to be bad for performance) with more performant (Bento) AMP components as part of the Page Experience Engine. In this case, we'd always want to stick with the latest stable version, but have the generated logic be adapted as needed with version changes.

[samouri]

So in this case the transformer needs to have a “best guess” of which component version to use.

You know better than I do, but my take is that this feature itself should be deprecated for the same reason that we decided to deprecate the -latest binary concept. Automatically upgrading to versions that that break compatibility seems like a bad idea.

That said, adding in experimental: true keys to the bundles.config.extensions.json is easy to do!

[westonruter]

An alternative route taken by ampproject/amp-toolbox#1273 is to captiure a snapshot of the current latest versions and to use them instead. I also did this as a stop gap measure in ampproject/amp-wp#6803. There is a new JSON file in the amphtml repo which has the legacy latest versions defined, introduced by #37177.

[samouri]

@westonruter: would you still like an experimental label on AMP extensions, or do you think pinning to versions is a more sustainable solution? As a defensive measure, some version of pinning (or at least a separate auto-import-versions.json5 file in amphtml) makes sense to me.

[westonruter]

@samouri Having a declarative way to find out which extensions are experimental seems like a win regardless. This would eliminate having to look through JS files to find imperative checks for experimental flags (e.g. #37364).

Nevertheless, now that latestVersion has been restored in #37355 then it seems we're back to where we started. Although is that going to be the frozen legacy latest or will it be the latest-that-is-not-experimental?

[jridgewell]

Although is that going to be the frozen legacy latest or will it be the latest-that-is-not-experimental?

I think the summary still holds, having a -latest that changes to the newest release (a breaking change) isn't useful. We should lock the "latests" at the current definitions, ensure they cannot be changed, and work to getting toolbox to use the separate json files. Even if we can't remove the latests from the bundle.config.json, it's still prevents us from causing breaking changes to users.

[samouri]

+1 to @jridgewell. The -latest version binaries are still frozen, and the latestVersion within the bundle.config.json will never be updated. We should still upgrade the various optimizers to stop depending on latestVersion key existing within the json and to instead point to the legacy jsonc file.

The primary impact of this recent discover/P0 is that we can likely never remove the pre-existing latestVersion keys from
bundles.config.extension.json.

[westonruter]

It also seems sensible to remove latestVersion from bundles.config.extension.json because currently they are duplicated across each version of a component, which is hard to work with (and which makes my eyes hurt). Better to have them exclusively located in bundles.legacy-latest-versions.jsonc where they can be normalized and more easily looked up (forever).

[schlessera]

I think the summary still holds, having a -latest that changes to the newest release (a breaking change) isn't useful. We should lock the "latests" at the current definitions, ensure they cannot be changed, and work to getting toolbox to use the separate json files. Even if we can't remove the latests from the bundle.config.json, it's still prevents us from causing breaking changes to users.

Just to provide another perspective here:
I currently see the latest version as being the "latest stable" version, as we also have experimental versions.

So while I agree that it is not a good idea to have your production site rely on a moving target version, there are other valid use case for providing a way to query the "latest stable" version:

• Running automated tests against the latest stable version to catch problems that an update would cause early;
• Query whether the currently locked version is different from the latest stable version to know when updates are needed;
• Direct people (in generated docs, through redirects, ...) to the correct version to start new development against.

I don't think a latest binary is a good idea. But I do think the concept of having a source of truth for the "latest stable version" is a valid and needed one.

[westonruter]

I don't think a latest binary is a good idea. But I do think the concept of having a source of truth for the "latest stable version" is a valid and needed one.

Yeah, and that could be addressed by having an experimental flag on versions in bundles.config.extension.json that are not stable.