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

Fix for "Browser console warning when beginning checkout" #3980

Merged

Conversation

4 participants
@prinzdezibel
Copy link
Contributor

prinzdezibel commented Mar 12, 2018

Resolves #3972
Impact: major
Type: bugfix

Issue

Receive the following warning in the browser console when clicking on the "checkout" button from the PDP. Also a spinner is shown for a beat or two. Seems to have no effect on actual functionality.

Note that this issue does not occur on every machine. It happen's reliably on Brent's machine, though. I was able to replicate it after playing with setTimeout times.

Solution / Changes

Don't call setState in a timer without prior check of mount state.

Breaking changes

none expected

Testing

  1. As an admin navigate to a product page
  2. Click on "Add to Cart"
  3. Click on the "checkout" button that appears briefly
  4. Observe no warnings in the console.
Fix for "Browser console warning when beginning checkout"
Resolves issue #3972

Changes:
Don't call setState in a timer without prior check of mount state.

@prinzdezibel prinzdezibel requested a review from jshimko Mar 12, 2018

@zenweasel zenweasel requested review from zenweasel and removed request for jshimko Mar 12, 2018

@zenweasel
Copy link
Member

zenweasel left a comment

Tested. Verified fixed.

@spencern

This comment has been minimized.

Copy link
Member

spencern commented Mar 14, 2018

@aldeed is this the right approach to silencing the
Warning: Can only update a mounted or mounting component.?

@aldeed

This comment has been minimized.

Copy link
Member

aldeed commented Mar 14, 2018

Yes, this is the approach we've been using. Technically every setState that's in a callback or promise chain should be wrapped in the isMounted check. For example, the one in the Meteor.call callback in handleAddToCart. As a general rule, whenever we add isMounted to a component, we should fix all of the async setState calls in that component.

@spencern spencern changed the base branch from release-1.9.0 to release-1.10.0 Mar 15, 2018

@spencern spencern merged commit eb422f3 into release-1.10.0 Mar 15, 2018

3 checks passed

WIP ready for review
Details
ci/circleci Your tests passed on CircleCI!
Details
security/snyk No dependency changes
Details

@spencern spencern deleted the fix-3973-prinzdezibel-fix-browser-console-error branch Mar 15, 2018

@spencern spencern referenced this pull request Mar 15, 2018

Merged

Release 1.10.0 #4013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.