Reaction: Sign in and Sign out not working properly in dropdown.

Created on 22 Sep 2017 · 20Comments · Source: reactioncommerce/reaction

Expected behavior

I should be able to sign in and sign out smoothly. You should sign in have spinner if necessary and go directly to your store with no flashing weirdness.

When I sign out it should simply return me to the signed out version of the store.

Actual Behavior

When I sign in with the dropdown, the full sign in page flashes behind it producing a very rough looking sign in.

When I sign out it flashes the full sign out page (this should all just happen in the dropdown). Then it takes me the store. After a longish pause it flashes the sign out page again, then starts spinning infinitely.

Steps to Reproduce the Behavior

Happens in regular browser but does not happen in incognito mode.

Versions

Node: 4.5.0
NPM: 5.4.1
Meteor Node: 4.8.4
Meteor NPM: 4.6.1
Reaction CLI: 0.12.0
Reaction: 1.5.0
Reaction branch: marketplace

bug

Source

rymorgan

All 20 comments

rymorgan on 22 Sep 2017

@rymorgan, I tried reproducing the bug but I didn't notice any flash. However, I observed that when the user tries to sign out from profile page, the user is not redirected to the home page.

rc-sign-out

Version

Node: 8.3.0
NPM: 5.3.0
Meteor Node: 4.8.4
Meteor NPM: 4.6.1
Reaction CLI: 0.10.0
Reaction: 1.5.0
Reaction branch: marketplace

efalayi on 25 Sep 2017

I'll reset my install and see if I can reproduce the error. If I can't, then I'll close the issue.

rymorgan on 25 Sep 2017

Not seeing all the issues described, but one thing I am noticing is the loader in the navbar. We should remove that loader icon from the sign in. There is already an icon on the page, and if it's in the navbar too, it pushes everything down awkwardly.

kieckhafer on 25 Sep 2017

I'm still seeing it after resetting my install, Mike can reproduce as well. Sometimes you have to sign in/ sign out multiple times.

rymorgan on 25 Sep 2017

It doesn't happen every time, but after a couple of tries, I get an infinite spinner when I sign in / sign out on the PDP page.

Also, on routes other than / I get a flash of the "sign in" form in the page as my route permissions are changing between user groups.

In addition, with marketplace shops, sometimes signing in as a marketplace shop, after previously signing out as a different user, causes a blank screen.

All in all, I think it's an issue with the auth, router startup on auth change, publications, and some timing.

screen shot 2017-09-25 at 11 26 56 am

mikemurray on 25 Sep 2017

See: https://github.com/reactioncommerce/reaction/pull/2752#issuecomment-332012801 -> regarding different handling of inline vs dropdown logins.

aaronjudd on 25 Sep 2017

@aaronjudd This issue is more about a bug that when you sign out as an admin you sometimes see a never-ending spinner or a blank screen.

But I did make an issue to get the defaults correct for sign in dropdown vs inline per your note.
https://github.com/reactioncommerce/reaction/issues/2944

rymorgan on 26 Sep 2017

Looking into this now.

mikemurray on 27 Sep 2017

The gray screen of death:

TypeError: Cannot read property 'brandAssets' of undefined
    at Brand.getLogo (brand.js:23)
    at Brand.render (brand.js:33)
    at modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108730
    at measureLifeCyclePerf (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108010)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108729)
    at ReactCompositeComponentWrapper._renderValidatedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108756)
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108680)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108658)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108579)
    at ReactCompositeComponentWrapper.receiveComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108481)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'brandAssets' of undefined
    at Brand.getLogo (brand.js:23)
    at Brand.render (brand.js:33)
    at modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108730
    at measureLifeCyclePerf (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108010)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108729)
    at ReactCompositeComponentWrapper._renderValidatedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108756)
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108680)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108658)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108579)
    at ReactCompositeComponentWrapper.receiveComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108481)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'brandAssets' of undefined
    at Brand.getLogo (brand.js:23)
    at Brand.render (brand.js:33)
    at modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108730
    at measureLifeCyclePerf (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108010)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108729)
    at ReactCompositeComponentWrapper._renderValidatedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108756)
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108680)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108658)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108579)
    at ReactCompositeComponentWrapper.receiveComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108481)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'brandAssets' of undefined
    at Brand.getLogo (brand.js:23)
    at Brand.render (brand.js:33)
    at modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108730
    at measureLifeCyclePerf (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108010)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108729)
    at ReactCompositeComponentWrapper._renderValidatedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108756)
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108680)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108658)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108579)
    at ReactCompositeComponentWrapper.receiveComponent (modules.js?hash=daca1da47146b5a1a1f456dbe7820eb1193c45fc:108481)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'toLowerCase' of undefined
    at ReactiveDict.ReactionProduct.getTag (products.js:299)
    at composer (productsContainer.js:140)
    at tracker.js:19
    at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
    at Tracker.Computation._recompute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:358)
    at Object.Tracker._runFlush (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:532)
    at onGlobalMessage (meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:398)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'toLowerCase' of undefined
    at ReactiveDict.ReactionProduct.getTag (products.js:299)
    at composer (productsContainer.js:140)
    at tracker.js:19
    at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
    at new Tracker.Computation (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:229)
    at Object.Tracker.autorun (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:613)
    at tracker.js:17
    at Object.Tracker.nonreactive (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:640)
    at tracker.js:16
    at Container._subscribe (compose.js:85)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker recompute function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: Cannot read property 'language' of undefined
    at startup.js:72
    at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
    at Tracker.Computation._recompute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:358)
    at Object.Tracker._runFlush (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:532)
    at onGlobalMessage (meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:398)

mikemurray on 27 Sep 2017

The underlying issue is that shop is sometimes undefined when signing out for a split second, and it seems a lot of things assume shop will always be defined.

mikemurray on 27 Sep 2017

👍1

@mikemurray maybe this fix might resolve #2750 as well? There's mention of a brandAssets error there too. @aanchirinah tried fixing it, but hasn't resolved it yet.

impactmass on 27 Sep 2017

@impactmass It's not only brand assets, its 18n, currency, anything client side accessing the shop without a check.

On my end, I've cleared all the "errors" but I'm still getting the grey screen after a few sign out attempts.

mikemurray on 27 Sep 2017

@mikemurray , Yes I also get the blank grey screen when I resolve the errors too when I try to sign in and sign out in approx. 2 cycles consistently.

aanchirinah on 27 Sep 2017

@impactmass @aanchirinah Confirmed that shop is undefined when the grey screen appears. On index ( / ) I get a grey screen. On /tag/shop I get "Oops! You don't have permission to view this content." sign in form.

screen shot 2017-09-27 at 10 46 57 am

mikemurray on 27 Sep 2017

👍1

Another thing I want to point out is doing reactive things in presentational reaction components.

// In brand.js inside getShop()

return Shops.findOne(Reaction.getShopId());

find, findOne and other reactive stuff belong in container components or trackers. React has no concept of Meteor's reactive nature.

mikemurray on 27 Sep 2017

In the router startup.js, if I place the needed subs there, everything seems to work ok-ish. Sign in feels a little slower, but it works every time.

This may be an issue with the subscription manager and how it attempts to cache subscriptions.

  const sub = Meteor.subscribe("PrimaryShop")
  const sub2 = Meteor.subscribe("Packages")
  const sub3 = Meteor.subscribe("MerchantShops")

  Tracker.autorun(function () {
    // initialize client routing
    if (sub.ready() && sub2.ready() && sub3.ready()
        // This was how it was done before with the subs mamager
        // Reaction.Subscriptions.Packages.ready() &&
        // Reaction.Subscriptions.PrimaryShop.ready() &&
        // Reaction.Subscriptions.MerchantShops.ready()
      ) {

mikemurray on 27 Sep 2017

@rymorgan @impactmass added a PR here https://github.com/reactioncommerce/reaction/pull/2966

mikemurray on 28 Sep 2017

Should this issue be closed since the PR is merged?

aaronjudd on 29 Sep 2017

yes. I missed the merge, but it was fixed when I tested it in the PR #2966

impactmass on 29 Sep 2017

Was this page helpful?

0 / 5 - 0 ratings