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

🐛 [amp-render]: fix race condition between renders #35584

Closed
wants to merge 4 commits into from

Conversation

dmanek
Copy link
Contributor

@dmanek dmanek commented Aug 9, 2021

Fixes #35484

Fixes race condition where rendered data is then overwritten by another render with empty data. Therefore, we check if data has mutated before rendering. It's weird that this issue is only reproducible in compiled mode (locally run amp --compile to repro).

@dmanek dmanek requested a review from alanorozco August 9, 2021 23:58
Copy link
Member

@alanorozco alanorozco left a comment

Choose a reason for hiding this comment

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

Looking for clarification on one thing, and a request to make a comment. I trust that you'll add the comment before merging.

extensions/amp-render/1.0/component.js Outdated Show resolved Hide resolved
extensions/amp-render/1.0/component.js Show resolved Hide resolved
@dmanek dmanek enabled auto-merge (squash) August 10, 2021 20:32
@dmanek
Copy link
Contributor Author

dmanek commented Aug 13, 2021

Closing this PR since the issue is been fixed by #35449.

@dmanek dmanek closed this Aug 13, 2021
auto-merge was automatically disabled August 13, 2021 18:30

Pull request was closed

@rsimha rsimha deleted the 35484-fix-amp-render-null-render branch August 27, 2021 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setState not working on slideChange of carousel inside amp-render
2 participants