search

visionmedia / page.js

Micro client-side router inspired by the Express router
http://visionmedia.github.com/page.js
7.67k stars 687 forks source link

Cannot read property 'bind' of undefined #501

Closed center-777 closed 5 years ago

center-777 commented 5 years ago

After upgrade to 1.10.0 i got an error: image

https://github.com/visionmedia/page.js/blob/v1.10.0/index.js#L53

matthewp commented 5 years ago

Hm, that is unexpected. Can you go to that line and let me know what this is?

flootr commented 5 years ago

We experience the same error. this seems to be defined correctly defined but _onpopstate is undefined at this point and therefore can't be bound.

The only thing I could spot why that might be is the following piece of code:

  Page.prototype._onpopstate = (function () {
    var loaded = false;
    if ( ! hasWindow ) {
      return;
    }

which evaluates to false and returns undefined instead of a function which could be bound.

Update: for our tests this only happens if they run without @jest-environment jsdom which provides a dom environment and defines a window object.

matthewp commented 5 years ago

@flootr Can you confirm that returning a function there instead fixes it? return function(){}; should be enough.

flootr commented 5 years ago

@matthewp that does indeed help. Can't speak for the issue author though.

nthibert commented 5 years ago

When will this actually make it out. When I do a npm install for my project, I get 1.10.0 down and I get this error as well. I know it was fixed but just wondering when this will make it live?

matthewp commented 5 years ago

I'll do a release today.

matthewp commented 5 years ago

Released in 1.10.2

flootr commented 5 years ago

@matthewp not sure what went wrong but 1.10.2 doesn't seem to include that fix. Ha, it seems my PR from above only fixed it for page.js, not index.js and index.mjs 🤔

matthewp commented 5 years ago

Oh, sorry I didn't look at that closely enough. page.js and page.mjs get generated from index.js. So changes should be made to index.js. I'll reopen since this is not fixed.

nthibert commented 5 years ago

Thanks for looking into this.