fix(connector): authEndpoint backward compatibility

[021-projects]

Fix #408

Issue source: https://github.com/pusher/pusher-js/blob/37b057c45c403af27c79303123344c42f6da6a25/src/core/config.ts#L151

[crynobone]

Should we add peerDependency for pusher-js?

[021-projects]

Should we add peerDependency for pusher-js?

I don't think so, because this dependency is currently optional, since Laravel Echo can use different connectors and Pusher may not always be needed. If we import Pusher from a dependency, it will increase the final bundle size for those using the socket.io connector. It would be nice to add type hints for options in the future, which would require add pusher-js, but that's later

[021-projects]

Maybe we should add a warning for deprecated options to the console?

[kirkbushell]

As good as it is to see this PR, this appears to break the standards and conventions set up within package, using ES6 classes instead of functions to manage the complexity. It would appear that moving those options out of that class is a mistake, and should be managed as part of the Connector API, as that manages the options.

I think it's a relatively straight-forward change - if authorizer is present as part of the options, use that, else default to the other options for connectivity.

[021-projects]

As good as it is to see this PR, this appears to break the standards and conventions set up within package, using ES6 classes instead of functions to manage the complexity. It would appear that moving those options out of that class is a mistake, and should be managed as part of the Connector API, as that manages the options.

In essence, options are still managed by classes - they just use utilities to improve code organization, which already happened with the isConstructor utility. Do you think this is a fundamental change? It seems like it's just code reuse. This makes the PusherConnector look much cleaner while still maintaining the same functionality. Also, the authorization options were moved to PusherConnector because Connector is an abstraction that is also used by other connectors that do not support Pusher options, which means that the abstraction only has code for one specific case. I thought this needed to be improved.

I think it's a relatively straight-forward change - if authorizer is present as part of the options, use that, else default to the other options for connectivity.

It's not just the authorizer, but several other options that stop working when the channelAuthorization property is provided, so the commit fad5599 essentially breaks the authorization functionality entirely, despite the fact that the properties have been deprecated for almost three years.

[taylorotwell]

This feels like a really big change?

[taylorotwell]

@ProjektGopher can you take a look at this if you have a chance?

[021-projects]

This feels like a really big change?

Not really big, just some refactoring to move Pusher options to the Pusher connector and fix backward compatibility

[taylorotwell]

Reverted the original PR for now. cc @ProjektGopher

[ProjektGopher]

Reverted the original PR for now. cc @ProjektGopher

That's what I was going to suggest as the easiest temporary solution. I agree that this PR looks like more work than necessary for now