auth0 / lock

Auth0's signin solution
https://auth0.com/docs/libraries/lock
Other
1.13k stars 556 forks source link

Doesn't work with React 16 #1148

Closed chapati23 closed 7 years ago

chapati23 commented 7 years ago

Versions

Lock version: 10.23.1 Browser: Chrome 61.0.3163.100 OS: macOS 10.13

Description

I've upgraded my app to React v16 today. Only package that breaks seems to be auth0-lock. If I downgrade to the latest React v15 then my app works fine. The error message isn't super helpful unfortunately but it seems to crash somewhere in the <Chrome> component. The <TransitionGroup> component mentioned in the error is just my app wrapper. Removing it doesn't change anything, same error.

Can only guess but the answer is likely in the Upgrade Section of the React 16 blog post.

Error


Check the render method of `Chrome`.
    in Chrome (created by Container)
    in div (created by Container)
    in form (created by Container)
    in div (created by Container)
    in div (created by Container)
    in Container

Screenshot

screen shot 2017-10-15 at 18 57 21
luisrudge commented 7 years ago

@chapati23 we're trying to figure it out how to help react@16 users. The ideal would be to move react and react-dom to peerDependencies, but this would be a breaking change for most of our customers, so we decided to pin 15.6.2 as a dependency for now.

In the meantime, react@16 will work. You'll just have to bundle both versions if you're not using yarn. If you are using yarn, however, you can use the resolutions field and pin react@16 to your repo. I created a repo that you can see how to use the resolutions field with yarn, so you don't have two react versions in your app: https://github.com/luisrudge/lock-react16

luisrudge commented 7 years ago

By the way, I came across the same error when I was upgrading Lock to work with react@16. I imagine you're using react-transition-group@v1, right? There's a breaking change to v2, and Lock is now using v2. I used this guide to migrate from 1 to 2: https://github.com/reactjs/react-transition-group/blob/master/Migration.md

chapati23 commented 7 years ago

gotcha, resolutions did the trick for me, thanks!

jch254 commented 7 years ago

resolutions did the trick for me too, cheers!

danrasmuson commented 6 years ago

Hit this issue as well. Resolutions FTW.

In this thread a react core member mentions this is a problem with connected libraries. Maybe this issue should be reopened?

pimterry commented 5 years ago

I'm using npm, not yarn, and I'd rather not switch just for Lock. To use this module, I've had to downgrade my version of React to 16.6.3, because it now seems to reliably implode in my application if used in conjunction with React 16.7.0 (the latest).

In the above link, the React official position is clearly that directly depending on React like this is incorrect. Are there plans to make the breaking change and move this to a peer dependency?

pimterry commented 5 years ago

Ok, I was wrong, it actually breaks for me with 16.6.3 as well, it's just totally broken.

For now, I've forked and republished this library, but with the react dependencies moved to peer dependencies. If you're in the same place, feel free to to use the fork, it should fix this immediately: https://www.npmjs.com/package/@httptoolkit/auth0-lock.

I'd much rather use the official version, so I'd love to hear when this issue is resolved.

luisrudge commented 5 years ago

In the above link, the React official position is clearly that directly depending on React like this is incorrect. Are there plans to make the breaking change and move this to a peer dependency?

The problem with this is that we need a major version for this kind of upgrade and we also need to update docs, quickstarts, readmes etc. The next major version will absolutely have this situation fixed, but we have other things that we want to add to the next major upgrade, so we can't fix this just now.

Although I can't force you to not publish this package, I will kindly ask you to not do that or do it with another name. We don't want to cause confusion to customers searching for our package and for some reason end up installing your version.

The simplest way to fix this is to just use our CDN build instead of using npm to load our package. This will decrease your final bundle size and you'll be able to only load Auth0 Lock in your login page.

pimterry commented 5 years ago

The first line in the readme calls it out as a fork, and it's a package scoped within my org (@httptoolkit), so I don't think it's too ambiguous. It's a common pattern elsewhere (you do it yourselves: @auth0/s3 is a patched fork of s3). I doubt there'll be any confusion.

I strongly prefer not to load JS from a CDN for a host of reasons (consistency with every other dependency, offline behaviour, extra CSP, etc). I don't have a specific login page either, as my app is a single page app, and the login widget is available at all times.

Right now unless I want to change package manager I can't use this module, and this still seems the nicest route. Because of that, it's useful to publish to npm so that other people in the same position can find & use it. At the moment this package breaks for anybody using npm and any version of React from the last year and a half, which I suspect is close to a majority of SPA apps, it's not a small group.

Bit of an aside, but why your CDN bundle would be smaller than my own bundling of this package with React removed? I'd rather not bring in CDN dependencies, but if there's further bundling options I should be looking at, I'm definitely interested. Are there any more details on that?

luisrudge commented 5 years ago

Bit of an aside, but why your CDN bundle would be smaller than my own bundling of this package with React removed? I'd rather not bring in CDN dependencies, but if there's further bundling options I should be looking at, I'm definitely interested. Are there any more details on that?

Sorry if I wasn't clear, I wanted to say that your final bundle would be smaller because you'd remove the auth0-lock dependency from it (but the final size of all scripts combined would be bigger, of course 😬)

pimterry commented 5 years ago

Ah, ok, sure. No worries, I was just wondering if I'd missed some a free way to shrink my bundle somewhere! Thanks.

pimterry commented 4 years ago

@luisrudge @davidpatrick @stevehobbsdev Are there any updates on this React dependency issue?

I'm still maintaining my forked version of this library (https://github.com/httptoolkit/lock), because this issue makes the official version unusable.

There's 100+s of other installs of my fork every week (not by me), and increasing, so this is clearly an problem for many other people too: https://www.npmjs.com/package/@httptoolkit/auth0-lock.

There's also many other bugs filed that are caused by this issue:

The only workaround available right now requires using Yarn, or using my fork. I don't want to be maintaining an important auth library for these users (and I doubt you want me to do so either) - I'd happily kill that fork if this issue was fixed!

I understand this requires a major release, and that's inconvenient. Is there any more detailed information available about why that's a problem? What're the direct consequences of bumping the version to do this?

deviantfero commented 4 years ago

why is this issue closed? I wish I didn't have to use this fork as it will probably not be maintained

stevehobbsdev commented 4 years ago

why is this issue closed?

It was closed because the original question was answered: use Yarn Resolutions to get the version of React that you need.

@pimterry Thanks for letting us know about your situation. We are working on some internal discovery around this now to figure out the best path forward, and have taken your comment into account. Hopefully I will have something to share reasonably soon.

nkemcels commented 3 years ago

2021 and the latest version of auth0-lock still uses React 15.

stevehobbsdev commented 1 year ago

👋🏻 We now have a v12 beta version that includes an upgrade to React 18 - please see this issue and let us know what you think, or if you encounter any problems!