Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore V1 elements in the inabox resource manager #32948

Merged
merged 1 commit into from
Feb 27, 2021

Conversation

dvoytenko
Copy link
Contributor

Partial for #31915.

This is a complete symmetry with the main Resources manager. See #32568 for more details.

Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think we should leave this alone?

@dvoytenko
Copy link
Contributor Author

I actually think we should leave this alone?

Ideally it'd work the same way in the inabox. But the main danger this avoids: V1 shouldn't have their layoutCallback called. I can work it around in the CustomElement (e.g. layoutCallback() { if (V1) return Promise.resolve(); ...}. But then why even call it?

Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, ok.

@dvoytenko dvoytenko merged commit 1a6b196 into ampproject:master Feb 27, 2021
@dvoytenko dvoytenko deleted the run3/deferred-build-inabox branch February 27, 2021 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants