CodeByZach / pace

Automatically add a progress bar to your site.
https://codebyzach.github.io/pace/
MIT License
15.68k stars 1.9k forks source link

Conflicts with zone.js? #399

Open rweng opened 7 years ago

rweng commented 7 years ago

Hi,

I'm not sure what is going on in pace, but in zone.js there is a function canPatchViaPropertyDescriptor, which basically does the same as the following test-snippet for the console:

function canPatchViaPropertyDescriptor(){
const xhrDesc = Object.getOwnPropertyDescriptor(XMLHttpRequest.prototype, 'onreadystatechange');

  // add enumerable and configurable here because in opera
  // by default XMLHttpRequest.prototype.onreadystatechange is undefined
  // without adding enumerable and configurable will cause onreadystatechange
  // non-configurable
  Object.defineProperty(XMLHttpRequest.prototype, 'onreadystatechange', {
    enumerable: true,
    configurable: true,
    get: function() {
      return true;
    }
  });
  const req = new XMLHttpRequest();
  const result = !!req.onreadystatechange;
  // restore original desc
  Object.defineProperty(XMLHttpRequest.prototype, 'onreadystatechange', xhrDesc || {});
  return result;
}

canPatchViaPropertyDescriptor();

When using pace-progress@1.0.2, following require changes the output from true to false.

const pace = require('pace-progress')

I am not 100% sure that it is a really a pace problem, since I am using webpack where I had to add the imports-loaderr@0.7.1 specifically for pace:

{
  test: require.resolve("pace-progress"),
  loader: "imports-loader?define=>false"
}

Anyway, I'll just remove pace from my project for now due to this issue combined with the added complexity of my webpack config. But maybe if someone is interested, you could check if you have the same problem without webpacks import-loader and if so forward the issue to the imports-loader repo.

All the best, Robin

MattJeanes commented 7 years ago

In the newest version of zone.js (0.8.12) using pace with it causes all XMLHttpRequests to fail with the error message:

TypeError: Unable to get property 'apply' of undefined or null reference

Please see my issue for more information. If I get time I'll try and fix the issue inside pace itself but I've already spent the good part of 3-4 hours trying to debug this issue so we'll see..

JiaLiPassion commented 7 years ago

@MattJeanes, The reason is it seems in https://github.com/HubSpot/pace/blob/master/pace.js#L351, extendsNative does not copy all properties, so may be you can try load pace.js before zone.js.

MattJeanes commented 7 years ago

@JiaLiPassion sorry for late reply but yeah turns out my tslinting decided to move the import order around so zone loaded after pace, a bit frustrating but all sorted!

adityamr15 commented 5 years ago

What I've done is just declare const key instead of key only in this line 354 FIXED my issue with zone.js hope also its fixed this issue #261