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

Using auth0-lock with React 16 #1096

Closed
1 task done
mdimovska opened this issue Aug 30, 2017 · 57 comments · Fixed by #1135
Closed
1 task done

Using auth0-lock with React 16 #1096

mdimovska opened this issue Aug 30, 2017 · 57 comments · Fixed by #1135

Comments

@mdimovska
Copy link

mdimovska commented Aug 30, 2017

React 16 is already added to the dependencies in package.json.
Currently, the dependencies in auth0-lock are specified like this:

"react": "^15.6.1 || ^16.0.0",

"react-dom": "^15.6.1 || ^16.0.0”

However there is not yet a release version of React 16, but only alpha and beta prereleases.
Since I am using React 16 alpha in the project, I get multiple installs of react:

auth0-lock@10.19.0
 └─ react@15.6.1
react@16.0.0-alpha.3

The reason for this is that prereleases are not considered if not specified explicitly, they are excluded from the matching.

Is it possible to append ^16.0.0-alpha and/or ^16.0.0-beta to react and react-dom, since there isn't a release of React v16 at the moment?

  • Lock version: 10.19.0
@luisrudge
Copy link
Contributor

We're not adding alpha/beta releases to our dependencies. We'll test lock with react 16 with it comes out officially.

@tim-soft
Copy link

tim-soft commented Sep 26, 2017

@landitus React v16 (and v15.6.2) has landed
https://facebook.github.io/react/blog/2017/09/26/react-v16.0.html

The annoying bit about forcing a specific version is in the case of a project that is using a newer version of React than Lock. Lock basically takes your app's React version hostage. A smaller dependency would be easy to work around but React versions can't be juggled within the same app.

@luisrudge
Copy link
Contributor

@tim-soft have you tested with react16? since we do accept v16, you shouldn't be getting two versions of react.

@tim-soft
Copy link

tim-soft commented Sep 26, 2017

I am about to do some testing with v16, that rant was more about unsupported releases such as the last few v16 betas/release candidates

@jeffijoe
Copy link

I just upgraded to v16 and I am getting this error in the production build only:

Element ref was specified as a string (container) but no owner was set. You may have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).

@luisrudge
Copy link
Contributor

Do you have a stacktrace?

@jeffijoe
Copy link

@luisrudge yes sir!

image

@luisrudge
Copy link
Contributor

ah, damn. only in production doesn't make a lot of sense for stacktraces haha. can you make this happen in dev mode?

@jeffijoe
Copy link

jeffijoe commented Sep 28, 2017

@luisrudge I've tried, can't reproduce in dev-mode. However I can tell you that the component it seems to be complaing about is the one that uses <div ref="container"...>

I have also checked the paths of the React version that is being called, which is the one at the root of my node_modules, so it's not like there are multiple copies of React.

@luisrudge
Copy link
Contributor

Alright, thanks. We use a few "ref" calls:
image

I'm finishing a major feature in another library and I'll be able to take a look on this on monday. If you can figure it out what's going on until then, I can cut a new release with the fix. Sorry I can't help you right away.

@luisrudge luisrudge reopened this Sep 28, 2017
@jeffijoe
Copy link

jeffijoe commented Sep 28, 2017

Yes that's quite a few ref calls, but the error explicitly states container is the ref that it is complaining about. So the file you should probably be looking at is container.jsx 😄

@luisrudge
Copy link
Contributor

When do you get the error? After you click something?

@jeffijoe
Copy link

Immediately when I invoke lock.show(...)

@luisrudge
Copy link
Contributor

After I upgrade to react@16 in this repo, I get an error after I click the signup button. but show works. are you starting lock in signup mode?

@jeffijoe
Copy link

No, here are my Lock opts if it helps:

  {
    theme: {
      primaryColor: '#A776E5'
    },
    auth: {
      sso: true,
      redirect: false,
      responseType: 'token',
      connections: ['omitted'],
      authParams: {
        scope: 'openid email user_metadata app_metadata picture'
      }
    }
  }

@luisrudge
Copy link
Contributor

ok thanks! I'll look into it.

@jeffijoe
Copy link

jeffijoe commented Oct 4, 2017

@luisrudge have you had a chance to look into it? I tried to pull the source and get it building with React 16 but no luck.

@luisrudge
Copy link
Contributor

Hi @jeffijoe. Still haven't found the time. Probably today or tomorrow. 🎉

@mattStegall
Copy link

We're also having an issue using Lock with React 16, specifically using the sign up portion of the pop-up.

Stack-Trace:

chrome.js:248 Uncaught TypeError: Cannot read property 'querySelector' of null
    at Chrome.findAutofocusInput (chrome.js:248)
    at Chrome.componentDidUpdate (chrome.js:206)
    at commitLifeCycles (react-dom.development.js:11517)
    at commitAllLifeCycles (react-dom.development.js:12294)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
    at commitAllWork (react-dom.development.js:12415)
    at workLoop (react-dom.development.js:12687)
    at HTMLUnknownElement.callCallback (react-dom.development.js:1299)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:1338)
    at invokeGuardedCallback (react-dom.development.js:1195)
    at performWork (react-dom.development.js:12800)
    at batchedUpdates (react-dom.development.js:13244)
    at performFiberBatchedUpdates (react-dom.development.js:1646)
    at stackBatchedUpdates (react-dom.development.js:1637)
    at batchedUpdates (react-dom.development.js:1651)
    at Object.batchedUpdatesWithControlledComponents [as batchedUpdates] (react-dom.development.js:1664)
    at dispatchEvent (react-dom.development.js:1874)

Looks like a ref that's being used isn't actually being defined. Was able to recreate using basic create-react-app configuration.

@luisrudge
Copy link
Contributor

@mattStegall I'm fixing this right now. I'll publish a new version today.

This was referenced Oct 6, 2017
@luisrudge
Copy link
Contributor

Hi @mattStegall, @jeffijoe, @tim-soft and @mdimovska.
I created a PR to upgrade to lock@16 (still being able to use 15.6.1) here: #1135
Can you guys please test it with your projects to see if it's working as expected?
If you want a CDN link: https://413-22887863-gh.circle-artifacts.com/0/home/ubuntu/lock/build/lock.min.js
If you want to use with npm:
yarn add auth0/lock#feature-upgrade-react
or
npm install auth0/lock#feature-upgrade-react

We plan to release a new version next week, after some testing. 🎉

@jeffijoe
Copy link

jeffijoe commented Oct 8, 2017

@luisrudge Tried with yarn, getting Module not found: Error: Can't resolve 'auth0-lock' — perhaps build artifacts are gitignored?

You can try to release with a dist-tag?

@luisrudge
Copy link
Contributor

@jeffijoe ahhh.. that's true. we don't publish the dist files in the repo. Can you clone that branch and use yarn link? That would be the best.

@mattStegall
Copy link

@luisrudge Cloned the repo and used your branch through yarn link and it works. 👍

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

@luisrudge did the same, however I wasn't as lucky as @mattStegall — I get the same error as before. 😢

@luisrudge
Copy link
Contributor

@jeffijoe this one? #1096 (comment)

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Well obviously I can't reproduce it in a clean repo, that would be too easy then wouldn't it? 😉

Unable to reproduce in a clean create-react-app I'm afraid. In my original project I have done everything to ensure only 1 copy of React is present: I've used yarn resolutions, I've ran npm ls react to check for potential dupes (all of which are reported extraneous), I've checked the Sources tab in Dev Tools and verified only one copy of react and react-dom are present.

The super odd part is that it's only an issue in production. 😢

@luisrudge
Copy link
Contributor

do you use CRA in this app?

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Not in my real app, no.

@luisrudge
Copy link
Contributor

I just removed ref="container" from the code. Can you see if this fixes your issue? (you'll have to pull latest)

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

I'll try it out right away — do I need to rerun yarn link etc? (never used it until now)

@luisrudge
Copy link
Contributor

I honestly don't know. Maybe it's best to unlink then link again? :>

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Been trying for 40 minutes now to get the yarn-linked package running in our staging environment, but every time I check the code with the container ref is still there. But when I run it locally, it works.

There's no way for me to verify this 100% unless I get a way to npm install the update. 😐

@luisrudge
Copy link
Contributor

So, locally with your codebase it works, right?

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Yes — going to try with the master branch to see if it works locally in prod-mode

@luisrudge
Copy link
Contributor

master doesn't have react@16 yet.

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Correct — but it will tell me if master would show the login box regardless — if the same error occurs, chances are that removing the ref might have fixed it.

@luisrudge
Copy link
Contributor

@jeffijoe Changes are now on master. Maybe it's easier for you to test this way?

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

willl that lead to a package publish?

@luisrudge
Copy link
Contributor

We'll publish a packge shortly. I'm merging a few more PRs before that.

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

@luisrudge silly me, I used yarn build and not yarn dist — after using yarn dist, the error still occurs, but with chrome instead of container as the cause of the error.

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Also, this still only occurs when using the production-build of React. If I manually UglifyJS the development build, it works fine.

@luisrudge
Copy link
Contributor

So, chances are there's something wrong with your build process, right? Because CRA's build process works just fine.

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

Mine isn't much different; I'll try to experiment with build config.

@jeffijoe
Copy link

jeffijoe commented Oct 9, 2017

@luisrudge HURRAY!!

I finally figured it out! Posting here to help other lost souls.


I tried to reproduce with a simple create-react-app and sure enough that worked. So I started tearing apart my webpack config until I found out the mismatch: the UglifyJS settings!

After removing the following 2 settings from compress, it seems to work:

unsafe: true,
unsafe_comps: true,

Remove those from your config and everything should work. Hope that fixes your issue as well!

@luisrudge
Copy link
Contributor

nice @jeffijoe!
thanks @jeffijoe and @mattStegall for helping us out with this migration. It should be live by the end of the day

@bryanvaz
Copy link

@jeffijoe & @luisrudge can you link to that React 16 workaround?

I forked auth0/lock and did a quick upgrade to react16 (https://github.com/imitadvisory/lock.git#react_v16_dist - using yarn so just committed a pre-built version to get moving) however I still get that annoying ref error referencing chrome. Did you add UglifyJS to Auth0/lock when you built it for React 16 or to webpack in create-react-app?

app_35d112da04e5aeb82c92.bundle.js:2266 Uncaught Error: Element ref was specified as a string (chrome) but no owner was set. You may have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).
    at invariant (app_35d112da04e5aeb82c92.bundle.js:2266)
    at coerceRef (app_35d112da04e5aeb82c92.bundle.js:29359)
    at reconcileSingleElement (app_35d112da04e5aeb82c92.bundle.js:30155)
    at reconcileChildFibers (app_35d112da04e5aeb82c92.bundle.js:30258)
    at reconcileChildrenAtExpirationTime (app_35d112da04e5aeb82c92.bundle.js:30379)
    at reconcileChildren (app_35d112da04e5aeb82c92.bundle.js:30370)
    at updateHostComponent (app_35d112da04e5aeb82c92.bundle.js:30621)
    at beginWork (app_35d112da04e5aeb82c92.bundle.js:30852)
    at performUnitOfWork (app_35d112da04e5aeb82c92.bundle.js:32847)
    at workLoop (app_35d112da04e5aeb82c92.bundle.js:32911)

Cheers,
Bryan

@jeffijoe
Copy link

@bryanvaz the issue only occured in prod, so it was my app-specific uglifyjs settings I changed.

@bryanvaz
Copy link

bryanvaz commented Dec 27, 2017

@jeffijoe How are you calling lock.show() to avoid the error?

I'm trying to build it up piece by piece so I'm just calling it from a function right below it, and it's whining about not having an owner:

const Login = (props) => {
  props.doAuthentication();
  return (
    <div>
      <h1>
        Login Area
      </h1>
      <ul>
        <li>AUTH0_CLIENT_ID: <b>{AUTH0_CLIENT_ID}</b></li>
        <li>AUTH0_DOMAIN: <b>{AUTH0_DOMAIN}</b></li>
        <li>Current Error: {props.error} </li>
      </ul>
      <div>
        <ul className="list-inline">
          <li><button className="btn btn-primary" onClick={onLoginClick} >Login</button></li>
        </ul>
      </div>
      <div id="auth-root" />
    </div>
  );
};

const onLoginClick = () => {
  lock.show();
};

@luisrudge there was some talk about adjusting the code to remove the refs in #1135. Which branch are those sitting on. I'm forking master right now, and all the original refs are still there. (Trying to do a react 16 upgrade without too many changes)

@jeffijoe
Copy link

Not sure what your props.doAuthentication() is for, but you shouldnt be calling side effect functions in render.

I use lock.show() as well.

@bryanvaz
Copy link

@jeffijoe it's just a ghetto check to see if the user is was authenticated (sidestepping a callback page for now since it's a hardcore SPA - no backend at all).

const doAuthentication = () => (dispatch) => {
  // If the User is authenticated using lock...
  lock.on('authenticated', (authResult) => {
    // Load the User's profile from the idToken in Lock
    console.log('User Authenticated!');
    // Use the token in authResult to getUserInfo() and save it to localStorage
    lock.getUserInfo(authResult.accessToken, (error, profile) => {
      if (error) {
        // Handle error
        console.log(`Auth0 Error: ${error}`);
        this.setState({ error });
        return;
      }

      localStorage.setItem('accessToken', authResult.accessToken);
      localStorage.setItem('profile', JSON.stringify(profile));
      localStorage.setItem('id_token', authResult.idToken);
    });
  });
};

@bkonkle
Copy link

bkonkle commented Jan 23, 2018

I'm having this same problem in a React 16 project - React complaining about adding a ref with no owner, likely due to the "2 copies of React" issue. I'm troubleshooting, and will share my solution here when I find one. 👍

@bkonkle
Copy link

bkonkle commented Jan 23, 2018

Solved with Yarn resolutions:

  "resolutions": {
    "react": "^16.2.0",
    "react-dom": "^16.2.0"
  },

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 a pull request may close this issue.

7 participants