🐛 Warn for amp-list[layout=container] without placeholder

[caroqliu]

Summary

amp-list with layout=container type should have a placeholder to function correctly. This PR knocks down the assertion to a warning, so that the element can continue building and laying out even if this requirement is not met. This is not harmful in a way that is end-user visible but can potentially lead to network requests on build for non-compliant usage forms, hence warning.

Additional Info

We originally required it so that the element is expected to have some initial height, and would not pop up out of no where on layout. But in fact, if there is no placeholder, diffable placeholder, or children otherwise (more on this in a bit), the amp-list will have a height of 0 and the runtime will not lay it out anyway. So there is already logic in place to ensure the user will provide placeholders for their component to work.

Finally - on the other children. This is not a standard way to use amp-list but it is AMP valid to do so, so it would be prudent to plan for this scenario I think. I tried out a few cases (placeholder & extra children nodes & template, and different subsets of these) with amp-list and it seems to behave reasonably given the unconventional use:

• placeholder is toggled on before layout and toggled off after layout
• list elements are appended and removed from an intermediary container element that amp-list creates, so other direct children of the amp-list are ignored during these operations
• in the layout=container case, we still measure initial height and ability to resize, and since the container element for amp-list elements gets appended after all user-provided markup, the user will not see even see a portion of that content unless there is a placeholder taking up space to replace

In particular, if you have <amp-list layout="container" ...></amp-list> or:

<amp-list layout="container" ...>
  asdf
</amp-list>

If I recall correctly, these are the main cases we had wanted to guard against (no placeholders) and they are not behaving visibly harmfully on the page with the assertion removed. The only cost is that both are built (potentially performing a network request), and the one with children (non-0 height) lays out which is potentially more costly to append that content, but again, the end user would not see it without a placeholder to replace. I think that is a reasonable situation to warn rather than error, and doing so should mitigate the user-visible errors you described in the original bug report when the component fails to find the placeholder.

Fixes b/178971515

cc/ @dmanek

[samouri]

While I agree this works for now, I prefer the assertion that amp-list must have placeholders to live within amp-list in addition to the implicit check via Runtime. The two are not obviously linked, and I'm worried it could cause future ux issues if we modified Runtime behaviors without reintroducing the amp-list check

edit: accidentally closed the PR. disregard that