Pull latest component versions from amphtml repo #1235
Conversation
sebastianbenz
force-pushed
the
improve-auto-import
branch
from
bc88513
to
24292f0
Compare
| this.bentoComponentInfo[bentoComponent.name] = bentoComponent; | ||
| } | ||
| } | ||
| this.componentVersions = params.componentVersions || {}; |
There was a problem hiding this comment.
Since optional chaining is supported in node 14, Would you ever want to use it rather than this type of syntax? Or do you prefer this type of format
There was a problem hiding this comment.
How does optional chaining help here? Do you mean nullish coalescing operator?
| @@ -15,7 +15,7 @@ | |||
| */ | |||
| 'mode strict'; | |||
|
|
|||
| const URL_BENTO_COMPONENT_INFO = 'https://amp.dev/static/bento-components.json'; | |||
| const URL_COMPONENT_VERSIONS = 'https://amp.dev/static/files/component-versions.json'; | |||
There was a problem hiding this comment.
Does this correspond to the latest version of each component?
There was a problem hiding this comment.
It seems some extensions are missing, namely:
amp-action-macroamp-base-carouselamp-cache-url(this lacks alatestin the spec)amp-google-assistant-assistjsamp-mega-menuamp-story-panning-mediaamp-stream-galleryamp-truncate-text
There was a problem hiding this comment.
Also, it doesn't seem accurate. It has this:
"amp-lightbox-gallery": "1.0",
And yet 1.0 is not even a valid version in the spec.
There was a problem hiding this comment.
The invalid latest version appears to be an issue with:
amp-facebook-comments(has 1.0 but should be 0.1)amp-lightbox-gallery(has 1.0 but should be 0.1)amp-twitter(has 1.0 but should be 0.1)
There was a problem hiding this comment.
Also, it seems bento-components.json is not complete. Namely, it is missing these Bento components:
amp-video-iframe(v1.0)amp-vimeo(v1.0)
There was a problem hiding this comment.
@westonruter Drive-by comment to suggest an alternative to https://amp.dev/static/bento-components.json:
- The source of truth for Bento components is in this file (search for
"npm": true). - These are the components that will be automatically published to npm (see design doc).
- In case it helps, we have a wrapper node script that can extract the raw list of components.
/cc @estherkim @jridgewell @kristoferbaxter for visibility / additional comment.
There was a problem hiding this comment.
@rsimha this is perfect! Exactly what I was looking for. Am I right, that latestVerstion will always point to the latest valid prod version?
Background: Bento components have been causing the problem here, but the real problem is that it's not possible to determine experimental components from the validation rules.
There was a problem hiding this comment.
Am I right, that latestVerstion will always point to the latest valid prod version?
Yep!
There was a problem hiding this comment.
Excellent! Updated the PR to use this correct endpoint.
One question: is it OK to simply strip the first two chars from an AMP release version to get the matching git tag? See https://github.com/ampproject/amp-toolbox/pull/1235/files#diff-87058858680239b88e937b9d753fde89d596c23045e56288ac0d4487e65f4567R113
There was a problem hiding this comment.
Yes! Git tags are created as part of the nightly cuts.
sebastianbenz
changed the title
|
Thanks everyone for the feedback and help! |
* Use different endpoint to fetch latest component prod versions * Use extension config from amphtml repo * update boilerplate error handler to fix validation errors
This will ensure that we'll always use the correct component version when auto importing scripts.