Skip to content

Fix infinite loop when uploading videos#3553

Merged
swissspidy merged 4 commits intodevelopfrom
fix/useEffectBug
Oct 18, 2019
Merged

Fix infinite loop when uploading videos#3553
swissspidy merged 4 commits intodevelopfrom
fix/useEffectBug

Conversation

@spacedmonkey
Copy link
Copy Markdown
Contributor

@spacedmonkey spacedmonkey commented Oct 18, 2019

Summary

Do not pass src in variable in the useEffect, as the function sets src and makes the code loop.

See #3463

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Oct 18, 2019
@swissspidy
Copy link
Copy Markdown
Collaborator

That fails the React hooks ESLint rule. That‘s why I disabled it in my proposed fix on the issue.

@spacedmonkey
Copy link
Copy Markdown
Contributor Author

That is my mistake. I did see that and I knew it was needed but forgot to copy it across. Fixed now.

@spacedmonkey spacedmonkey requested a review from miina October 18, 2019 12:41
@swissspidy swissspidy added this to the v1.4 milestone Oct 18, 2019
@swissspidy swissspidy changed the title Fix useEffect loop. Fix infinite loop when uploading videos Oct 18, 2019
@swissspidy swissspidy merged commit b8a8e11 into develop Oct 18, 2019
@swissspidy swissspidy deleted the fix/useEffectBug branch October 18, 2019 13:39
westonruter added a commit that referenced this pull request Oct 18, 2019
…twentytwenty-support

* 'develop' of github.com:ampproject/amp-wp: (73 commits)
  Mock HTTP request for Test_AMP_WordPress_TV_Embed_Handler
  Add test case
  Fix issues with image display (#3555)
  Fix infinite loop when uploading videos (#3553)
  Fix styling on remove button. (#3554)
  Test for XML processing instruction
  Undo over-eager rename of auto_accept_sanitization
  Rename auto-accept to accept-by-default
  Rename filter to amp_validation_error_auto_sanitized
  Fix jumping blocks when changing fonts (#3529)
  Update dependency browserslist to v4.7.1 (#3551)
  Make sure that isPageBlock always returns boolean.
  Fix and improve e2e tests.
  Remove unnessary check for block. Use shift over access first element of array.
  Use isPageBlock for cleaner code.
  Fix minor grammar issues
  Add docblock for `amp_is_sanitization_auto_accepted` filter
  Align arrows
  Update tests to use filter to set sanitization auto acceptance
  Make AMP_Validation_Manage::is_sanitization_auto_accepted() filterable
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants