crisward/riot-routehandler

Sends multiple requests, displays the data several times (2-4), between the transitions, how to fix it

Closed this issue · 14 comments

I send one request, but instead send two.

  var count = 0; //Announced the counter
  riot.control.on('postProfile', function(id_user, current, units) {
    var page = current || 1;
    var pages = units || 5;

    riot.httpx.request({
        url : riot.api + '/advert',
        method : "GET",
        data: {
          id_user: id_user,
          from: (page - 1) * pages,
          to: pages
        },
        dataType: 'json',
        success : function(data) {
          // The request must be worked out once, but ...
          count++;
          console.log('Сompleted', count); // returns Сompleted 1, Сompleted 2
          // works several times 
          self.items = data.result;
          self.update();
        }
    });
  });

Оutput on several times occurs with any data not only with requests.

Sorry, I'm not sure what this has to do with riot-routehandler? Can you explain more.

I use webpack to build the project, but I do not think this is the case. If you output data to the console when navigating the routes, you can see multiple calls to the same data. I came to the conclusion that this is due to updating, mounting and again updating the tag.
I found a way to reduce the amount of data on transitions, but still they stayed and it scares, and doing multiple repeated requests to the server is bad for the server.

this.setTag = (function(_this) {
return function(tagname, routeopts) {
var tag;
_this.update({
tagname: tagname
});
tag = riot.mount(tagname, routeopts); // mounting
tag[0].opts = routeopts;
tag[0].update(); // update
return tag;
};
})(this);

200 GET → 120 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 213 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 163 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 241 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 299 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 341 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 375 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 407 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100

The number of requests per transition

My version

this.setTag = (function(_this) {
return function(tagname, routeopts) {
// here update the tag
// I do not know how to fix it
_this.update({
tagname: tagname
});
// then mount the data output from here twice
// return riot.mount(tagname, routeopts); // If you uncomment the output of the data twice
// because of this decision we lose the context, but I would like to keep :)
return riot.util.vdom[0];
};
})(this);

200 GET → 77 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 111 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100
200 GET → 151 ms advert?id=30576197&user_filter=true&is_all=1&is_active=0&is_archive=0&from=0&to=100

Decreased six-fold

There are tests in this component that check for mounts being called multiple times. If you're using hot-reloading as part of your development build, that could be the cause. I haven't used webpack with this component so I can't give advice specifically on that.

If you want to stop multiple dataloads, perhaps set a variable outside your component scope, which records that a load has been started, then only call load if this is not set. This should prevent multiple calls.

I'm sorry I can't offer more help. Perhaps share a more complete example in a github gist.

I returned with the solution of this question. Having studied the mounting of tags in more detail, I found a bug in the first line.
riot.tag2('routehandler', <div data-is={tagname}></div>, '', '', function(opts) {...
The bug is to add a construct <div data-is={tagname}></div> when mounting the routehandler itself.
And when a riot finds such a construction, it automatically mounts the tag when it is updated, thus changing the code, we will get rid of the double mount and its consequences.

riot.tag2('routehandler', '', '', '', function(opts) {
...
this.setTag = (function(_this) {
  return function(tagname, routeopts) {
    var tag;
    // here there was an extra mounting
    // _this.update({
    //   tagname: tagname
    // });
    _this.root.innerHTML = `<div data-is="${tagname}"></div>`
    tag = riot.mount(tagname, routeopts); // here we pass options
    // tag[0].opts = routeopts; // superfluous transfer of options
    tag[0].update();
    return tag;
  };
})(this);
...
});

that's it, fix the routehandler

Thanks for digging into this. So you're basically suggesting removing this line
https://github.com/crisward/riot-routehandler/blob/master/src/routehandler.tag#L42

I'll give that a go later (at work at the moment) and check the tests still pass. This was originally designed for use with Riot v2, which I'm still using. The update to Riot 3 may be the underlying issue here as I'm sure some of the underlying behaviours of mounting and updating changed a little.

This should work and in riot 2, based on the documentation http://riotjs.com/v2/guide/#html-elements-as-tags.

Your suggested fix may solve the double mount, but it also caused middleware not to run and 17 tests to fail.

We do have a failing test due to a double mount. This must be due to a newer version of riot being used, compared to when I first developed this. I'll track down the change and come up with a fix.

All tests pass on 3.0.2 - so something has happened since then.

I've put a fix in place for this. Riot had updated to auto mount a tag when data-is is updated. Earlier versions of riot 3 didn't do this. However, I'm also manually mounting the tags to pass down the opts. So I've removed the update, and manually inserting the tag name into data is, before doing the manual mount.

It took me forever to get this working. Hopefully this fixes your issue.

Let me know if this master branch works, I'll then do another release to npm.

Thanks!

Yes it works, for my version of the riot 3.6.1. I had to fix the first line, because I left the second parameter blank.

-  riot.tag2('routehandler', '<div data-is="{tagname}"></div>', '', '', function(opts) { ... }
+ riot.tag2('routehandler', '', '', '', function(opts) {...}

I transfer the creation of a child node after mounting
Without this step, my improvement in redefining routers did not work

this.on('mount', (function(_this) {
 +  _this.root.innerHTML = '<div></div>';

and

this.setTag = (function(_this) {
  return function(tagname, routeopts) {
    var tag;
    _this.root.childNodes[0].setAttribute("data-is", tagname);
    _this.tags[tagname];
    tag = riot.mount(tagname, routeopts);
-   tag[0].opts = routeopts; // I think this is an extra transfer of parameters, we pass them on when mounting the tag
    return tag;
  };
})(this);

so everything worked

I had a need for resetting routs and assigning new ones, I added a slight improvement

this.on('mount', (function(_this) {
+ page.callbacks = [];
 _this.root.innerHTML = '<div></div>';

With this improvement, you can show different navigation routes, for example, when the user is not authorized to show the demo version of the site and vice versa

if(logged){
    routes = [
      { route: '*', use: start },
      { route: "/", tag: "main" },
      { route: "/pages", tag: "pages" }
    ]
} else {
   routes = [
      { route: '*', use: start },
      { route: "/", tag: "splash" },
      { route: "/demo", tag: "demo" }
    ]
}

this is a simplified example, I think that it is understandable

I'm guessing you're not running the unit tests for your changes, also you seem to be editing the compiled code. It's been written in coffee-script. See the src folder.

Just run npm install, then npm tdd while your changing your code.

The current version passes all the tests, several fail with your changes above. Feel free to add more tests for your use case. But the two sets of routes should be possible with an if and two router tags.

I'm not strong in the coffee-script, so I'll stay with the current changes, they work for me. I hope you can improve the routehandler. Thanks for the help. I think the issue is closed