VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor
http://vulcanjs.org
MIT License
7.98k stars 1.88k forks source link

Accounts button disabled #1815

Closed eric-burel closed 6 years ago

eric-burel commented 6 years ago

Hi, The account buttons are disabled in production (app here), though they are working if I manually disable them in the React console.

It seems that the Accounts.loginServicesConfigured() events does not happen, so the buttons are never activated (waiting state here).

I also have this empty collection in my DB: meteor_accounts_loginServiceConfiguration

The bug appeared, of course, totally widly, the app use to work correctly.

Root URL seems correct too: ROOT_URL: https://simply-charge-staging.herokuapp.com

luhagel commented 6 years ago

1816

Until the fix is deployed to Atmosphere etc, using a 2 repo approach to bundle the corrected version should do the trick

eric-burel commented 6 years ago

Ok, just to note, underlying issue for people that encounter this is that 1.8.3 has (small) breaking changes, and versions are not locked by default.

I think it would be good to lock all versions by default (with vulcan:foobar@=1.8.x), since Vulcan user are used to go directly in the package list files anyway.

SachaG commented 6 years ago

Just to clarify, is this a recent issue? Do you know when it was introduced, or what caused it?

luhagel commented 6 years ago

Pretty sure it's also somehow related to the React 16 SSR changes, and it only appears in minified production builds.

MHerszak commented 6 years ago

@luhagel can you clarify "related to ssr changes"? Accounts.loginServicesConfigured() is subscribing to meteor.loginServiceConfiguration and returns ready true when the subscription is ready. If ready, waiting turns false. Is there a bottleneck I don't see?

luhagel commented 6 years ago

Yeah, my suspicion is that there's a data mismatch between client and server, leading to one version overwriting the other with the incorrect waiting state

MHerszak commented 6 years ago

@luhagel or @eric-burel what is the current workaround for this? As mentioned the production build is in a waiting state and I am trying to find the cause. Any ideas for a workaround?

luhagel commented 6 years ago

@MHerszak bundle your local version with the build and replicate #1816

MHerszak commented 6 years ago

@luhagel Which package versions are you using? The latest version? Nvm, I get it now. Master hasn't been updated, yet. I was so confused about the versioning. Disregard my question :)

SachaG commented 6 years ago

I'm pushing 1.8.4 which includes this fix. Let me know if it helps.

MHerszak commented 6 years ago

The issue is resolved. I am closing this for now.

MHerszak commented 6 years ago

Nvm, state change isn't working anymore.

Discordius commented 6 years ago

Does anyone have any potential fixes for this? I've been running into the state change issue.

SachaG commented 6 years ago

In the meantime you can use different routes or handle the state changes yourself:

import { Components, registerComponent } from 'meteor/vulcan:core';
import React, { PropTypes, Component } from 'react';
import { FormattedMessage } from 'meteor/vulcan:i18n';
import Dropdown from 'react-bootstrap/lib/Dropdown';
import { STATES } from 'meteor/vulcan:accounts';
import { Link } from 'react-router';

const UsersLogIn = ({state}) =>

  <div className="page accounts-page accounts-log-in">
    <Components.AccountsLoginForm showSignUpLink={false}/>
    <p className="accounts-prompt"><FormattedMessage id="accounts.dont_have_an_account"/> <Link to="/sign-up"><FormattedMessage id="accounts.sign_up_here"/></Link></p>
  </div>

UsersLogIn.displayName = 'UsersLogIn';

registerComponent('UsersLogIn', UsersLogIn);
import { Components, registerComponent } from 'meteor/vulcan:core';
import React, { PropTypes, Component } from 'react';
import { FormattedMessage } from 'meteor/vulcan:i18n';
import { STATES } from 'meteor/vulcan:accounts';
import { Link } from 'react-router';

const UsersSignUp = ({state}) =>

  <div className="page accounts-page accounts-sign-up">
    <Components.AccountsLoginForm formState={STATES.SIGN_UP} showSignInLink={false}/>
    <p className="accounts-prompt"><FormattedMessage id="accounts.already_have_an_account"/> <Link to="/log-in"><FormattedMessage id="accounts.log_in_here"/></Link></p>
  </div>

UsersSignUp.displayName = 'UsersSignUp';

registerComponent('UsersSignUp', UsersSignUp);
SachaG commented 6 years ago

I've opened an issue: https://github.com/facebook/react/issues/12102

MHerszak commented 6 years ago

Yeah, I am handling this manually at the moment. Thanks for opening an issue, Sacha.

SachaG commented 6 years ago

Here's a fix for now: https://github.com/VulcanJS/Vulcan/commit/6facf15e17fec0eff0804d35360579513e7f586f

vincro commented 6 years ago

I had the same issue, which is now fixed by the update.

nolandg commented 6 years ago

I'm seeing this issue where setState is having no persistent effect on this.state within LoginFormInner and a5c3785ed8d6a35868bc169f07e40e889087fd2e does not fix this. ---Only in production mode :-( Works perfectly in dev

My input component correctly calls handleChange(field, evt) in LoginFormInner and that function calls setState with the correct values. Immediately after the setState call, this.state reflects the change but on the next call to handleChange the value is gone. As @SachaG said in his React issue, the callback for setState is never called in handleChange. The callback is called in dev but not production.

Any ideas? Can @SachaG explain why his above commit was supposed to fix this? Any guesses as to why it working in dev but not production?

Also, I'm noticing that my login form including the AccountsButtons are re-rendering on every onChange of my text input (password and username etc) in dev but not in production. The whole form seems dead in production.

nolandg commented 6 years ago

Ok. I think I figured it out. I say it's the fault of that tracker-component that LoginFormInner extends.

tracker-component was last updated 2 years ago. It's setState method relies on the undocumented private this._reactInternalInstance to decide how to update state. Which branch it takes in it's setState method changes based on whether you're in production or dev mode. I think the last 2 years of React updates have rendered this component obsolete and broken.

I have re-written the setState function of tracker-component to look like this:

  setState(state) {
    const newState = Object.assign({}, this.state, state);
    this.state = newState;
    return super.setState(newState);
  }

I removed the reliance on _reactInternalInstance and saved local state and called super for both cases. It's working for me in production and dev mode.

This is a small simple component. I propose we don't wait for a PR from it and simply include this component in vulcan-accounts. I could submit a PR for that next week if @SachaG agrees.

My login error messages stopped working though when I pulled that latest commit so I'll have to fix that too but I think that's unrelated and might be due to my implementation of AccountsMessage.

SachaG commented 6 years ago

Oh nice! Yeah this would be a huge help, a PR would be great. Thanks for figuring this out, I've been struggling with this issue for a while…

SachaG commented 6 years ago

@nolandg any update on this? would you have time to submit a PR?

nolandg commented 6 years ago

Sorry for the delay. Let me know if there's any problems with it. I reset to a clean devel and it works fine for me in production mode.