Strange behavior with ES6 synthax

[thedgbrt]

Hello @jpodwys, thank you for your great work on this, it's a very much needed module.

I'm pretty new to all things npm || webpack and I'm struggling to make your module work properly in my ES6 project (based on a react.js boilerplate).

Here's the simplified version of the file I'm working with ;

import superagentCache from 'superagent-cache';
import config from '../config';

const methods = ['get', 'post', 'put', 'patch', 'del'];
const superagent = superagentCache();

class _ApiClient {
  constructor(req) {
    methods.forEach((method) =>
      this[method] = (path, { params, data } = {}) => new Promise((resolve, reject) => {
        const request = superagent[method](path);
        request.end((err, { body } = {}) => err ? reject(body || err) : resolve(body));
      }));
  }
}

const ApiClient = _ApiClient;
export default ApiClient;

This was working perfectly before I added superagent-cache, and now the request returns an error.

I'm trying to figure out what's happening, and I've noticed that the content of error code matched what should have been the response if the call had succeeded. So I thought maybe the ES6 synthax is making superagent-cache not understand the function parameters in the correct order, and I tried switching this:

request.end((err, { body } = {}) => err ? reject(body || err) : resolve(body));

with this:

request.end((body, { err } = {}) => err ? reject(body || err) : resolve(body));

And the error was gone! The response data was correctly returned and I could get the result of my call like before.

So this is the 1st weird thing and I imagine that this is the cause of the next issue.

The 2nd one is that I can't seem to get backgroundRefresh() to work. I include it with no options and it doesn't do anything no matter how many time I refresh the page. I'm thinking this is because the response object is somewhat buggy and therefore it can't properly make the background call?

I hope I'm not wasting anybody's time here, I'm really looking to get this fixed so my app can fly :) Let me know if there's something I can do to help find a fix to these issues.

[jpodwys]

Thanks for the detailed report! As I understand it, these are the two issues you're reporting:

Issue 1

If the two code bodies below do the same thing, then I'll need to look into this some more before being able to comment on it. Not sure why, when using ES6, the err and response params would be returned in reverse order. Am I understanding the issue correctly?

request.end((err, { body } = {}) => err ? reject(body || err) : resolve(body));

request.end(function (err, response){
  (err) ? reject(err) : resolve(response);
});

Issue 2

I include it with no options and it doesn't do anything no matter how many time I refresh the page.

I assume you're using superagent-cache on the client. Please correct me if I'm wrong.

The intended use does not require you to reload the page. Consider opening your browser's network tab to discover whether the background refresh is indeed triggering a network event.

superagent-cache stores data and background refresh functions in its internal cache instance. You can use any of the available ones, but it uses cacheModule by default. cacheModule's backgroundRefreshInterval param defaults to 60 seconds. This means that, unless configured to behave differently, superagent-cache will only refresh designated keys every 60 seconds.

If you'd like to change that number and you're using the bundled cacheModule instance, you can provide a config object for superagent-cache to use when constructing cacheModule as follows:

var superagent = require('superagent-cache')(null, {backgroundRefreshInterval: num});

If you would like to see some code examples, you can look at the background refresh test file. The tests are set to execute a background refresh every 500ms to keep the tests quick.

If you believe you're using background refresh correctly, please provide a code snippet from your project so I can see what you're seeing.

Thanks!

[thedgbrt]

Thank you very much for the thorough answer, here are my remarks :

#21 Issue 1

I don't understand the way ES6 work so well that I'm able to help much here. My current environment (using a boilerplate) is full of linters and give me errors when I use your second snippet. I did try the following though (which I believe is equivalent) and it didn't return the same thing as the original swap I did.

request.end(function(err, response) {
  if (err !== undefined) {
    reject(err);
  } else {
    resolve(response);
  }
});

Here is the full file / link to the boilerplate I'm using : https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/helpers/ApiClient.js

Issue 2

Actually I'm using it on the server.
My use case is this : I make the API call on the server, and I wait for the response to start showing the page (for SEO purposes).
Caching works as it should, first page load after booting the app is slow (time to fetch data) but then every time I refresh is really quick. Now if I update some data in my API and hit refresh, I can't ever see it no matter how many time I refresh or how long I wait before doing so (tried the 500ms option to no effect).

[jpodwys]

Issue 1

If you're satisfied with that code, then I think your concern is resolved. Neither of us seem to know enough about ES6 or your specific usage to understand why the params were reversed. However, to me, the code you wrote just above this comment is far more readable than the code in your original post, so I'd just go with this.

Issue 2

If your code is open-source, can you please link me to the file you're working out of? If not, can you make an open-source gist or provide me with a way of viewing your code? I'd like to help, but I'm afraid I can't see what's going on. Thanks!

[thedgbrt]

Issue 1

I agree!

Issue 2

Here's the gist of the complete file
https://gist.github.com/dagobertrenouf/26ac3a4ad4a2aace384a

With this code I manage to get the data as expected, and the plugin you created caches it on first load. It just seems that backgroundRefresh() doesn't do anything, as I can't get any updated data, ever after first running the app. I feel like I'm not understanding how this function is supposed to work, especially when ran from the server like here.

[thedgbrt]

After digging into it more it seems that I misunderstood the purpose of backgroundRefresh. What I'm really wanting is cache invalidation as soon as data changes, I will follow this issue instead #20

[jpodwys]

Ok thanks for the update.

[jpodwys]

Hi, sorry I never followed up with you when the feature was officially added, but the forceUpdate feature has been in the master branch for a while now. Was just reading over some docs and remembered this issue. Happy coding!

[thedgbrt]

@jpodwys thanks for following up :)