Question: Patching superagent
Closed this issue · 6 comments
I have recently been doing some work on minimising the size of our client side bundle and realised that superagent-cache
comes with it's own version of superagent
, while we also have our own upto-date version of superagent
in the app. I know there is some planned work to have superagent
as peer dependency for superagent-cache
, so for the mean time to minimise our client bundle size, I am aliasing any require
call to superagent
to the version that is currently being used in the app using webpack and not the one superagent-cache
has as a dependency.
Reading the docs on superagent-cache
says that, it patches superagent
, so I am a bit worried that it could have unknown side effects because of the approach I am taking. So just wanted to have some clarification if this approach could end up having some unknown side effects. Thanks.
That's correct, it could indeed have unknown side effects. I have done no testing with superagent 2.x
. I would like to support it at some point, but I haven't had anyone ask for it and I suspect there are still far more people using superagent 1.x
than 2.x
, although I'm unable to prove it.
Are you using features specific to the version of superagent pulled in by your app? If you're not dependent on any features introduced since 2.x
, you might consider pinning your app to 1.7.1
or something like that.
If that solution is undesirable, you can try forking superagent-cache, pinning it to superagent 2.x
, and then running superagent-cache's unit tests to see what fails.
And, of course, you can officially request that superagent-cache support superagent 2.x
. That will, however, be quite a bit of effort and is therefore unlikely to be finished soon.
What are your thoughts?
So, I just pinned my local copy of superagent-cache to superagent 2.3.0
and all the tests passed. I'm quite surprised, to be honest. Perhaps we have less to worry about that we originally thought.
I should be clear, though--I don't believe superagent-cache has 100% test coverage, so it's likely that there are still scenarios in which forcing it to consume superagent 2.x
could cause problems. However, someone else told me they've been using it with superagent 2.x
for a little while and have not yet encountered issues.
Potential compatibility issues I'm thinking of might have to do with using actual promises and how headers are stored which could impact cache key generation.
Sorry for all the comments--just realized superagent has a 3.0.0
release as of 5 days ago. Tried my unit tests and all of them passed against it as well.
Thanks for your input. Apologies I was a bit vague when I said more up to date version of superagent
. I haven't actually kept track which is the up to date version of superagent
so thought the version we were using in our app which is 1.8.4
is the latest.
Reading through your comments and the docs it looks I should be okay with 1.8.4
.
The actual question I had was because superagent-cache
patches superagent
. When I do
const superagent = require('superagent')
anywhere else in the app, is the superagent
instance here now a patched version of superagent
.
To give more context my webpack config looks like below
....
resolve: {
alias: {
superagent$: path.resolve(__dirname, 'node_modules/superagent/lib/client.js')
}
}
....
which is basically looking at any require call for superagent
and providing it with the file I have specified in the alias. So for superagent-cache
a require call to superagent
will resolve to file mentioned in the alias. I am just a bit curious to know that if now I make a require call to require(superagent)
anywhere else in the app would the instance of superagent
I get be a patched one. Essentially I would want a non patched version to avoid any side effects going forward.
Please let me know if I am not making any sense, not sure if I am doing a good job of explaining my issue/thoughts.
I've reworded what I believe your question to be as follows:
superagent-cache patches superagent and superagent is a singleton. Does this mean that whenever I call
const superagent = require('superagent');
in my app I'll get the patched superagent instance?
The truth is that I don't know, but it seems like it would be a simple thing for you to check within your app. You can patch superagent then require superagent in a few different files and check for the superagent.cache
property. If you do check it out, please let me know your findings.
Can you please also help me understand why you want superagent-cache in some scenarios but not others? If it's simply to avoid caching certain data, you can use superagent-cache's ._end
function to bypass all caching logic. However, if updating many uses of .end
and/or coupling yourself that tightly to superagent-cache's API doesn't seem reasonable for your use case, I understand.
In the app we are not really using superagent-cache
, we have a dependency in the app, which pulls in superagent-cache
.
app
-src
node_modules
- package-that-pull-in-superagent-cache
node_modules
superagent-cache 1.7.1
node_modules
superagent 1.7.1
-superagent 1.8.4
So I thought may be I can minimise the bundle size by providing a single version of superagent
for the app
and superagent-cache
, which one of our dependency is pulling in. In a way it is a bit extreme optimisation but thought of giving it a try and it seemed to work.
Unfortunately for me the superagent
package ends up being patched for the app
as well. So for now I have reverted back the change. To be honest I just created a really unique scenario for myself.