Fix middleware backwards compatibility #246
Conversation
| client = client instanceof Raven.Client ? client : new Raven.Client(client); | ||
| return client.errorHandler(); | ||
| connectMiddleware.errorHandler = function (clientOrDSN) { | ||
| utils.consoleAlert('top-level Raven.middleware.* methods have been deprecated and will be removed in v2.0; use instance methods instead'); |
There was a problem hiding this comment.
"use instance methods instead"
Are people going to know what this is? Can we link to something here, or say the actual code?
Also, what's the harm in keeping this around?
There was a problem hiding this comment.
Sort of trying to keep that warning from being excessively long, but I'll be more specific about the methods I'm referring to.
Harm is that a user might just pass their DSN string in 3 places and end up with 3 instances, two of which are sort of hidden and probably entirely unnecessary, when they really just want one instance. Might result in situations where e.g. user expects to get breadcrumbs but doesn't. I want to avoid multiple instances entirely unless the user is explicitly asking for them and knows what they're doing.
There was a problem hiding this comment.
Good explanation – I agree. My fear is that people will see that message and have no idea what you're talking about, especially since it's not yet reflected in docs.
Can we do something like:
utils.consoleAlert('top-level Raven.middleware.* methods have been deprecated and will be removed in v2.0; use client.requestHandler()');There was a problem hiding this comment.
Yea, I updated it to roughly that. Adding a test.
* Fix middleware `client instanceof Raven.Client` check to use `Raven.constructor` (thanks @dhritzkiv) * Add deprecation warning to top-level middleware * Add tests for top-level Raven.middleware backwards compat
|
Published 1.0.1 and 1.1.1; thanks @dhritzkiv! |
instanceof Raven.constructorinstead ofinstanceof Raven.clientBased on #245 from @dhritzkiv. Will merge into master and also cherry pick into a
releases/1.0.xbranch, then publish 1.0.1 and 1.1.1./cc @benvinegar