IE 11: Object doesn't support property or method 'isView'

[JamieMason]

Hi,
I noticed in a bug report from Sentry that /index.js#L67 throws Object doesn't support property or method 'isView' in IE 11.

Broadly speaking, I think the fix would be to check for ArrayBuffer.isView before invoking it:

- if (hasArrayBuffer && ArrayBuffer.isView(a) && ArrayBuffer.isView(b)) {
+ if (hasArrayBuffer && ArrayBuffer.isView && ArrayBuffer.isView(a) && ArrayBuffer.isView(b)) {

I'm happy to submit a PR if you agree to this change.

Thanks.

[chrisbolin]

ugh IE again lol.

i'd welcome a PR! let's actually change the line var hasArrayBuffer = typeof ArrayBuffer === 'function'; and add a check for isView. It'd be nice to comment to note the IE fix. One of the goals of this package is to be very small, so try and think thru the minimal way to safely create hasArrayBuffer

[kitten]

Hm that’s odd... MDN flags ArrayBuffer.isView as supported on IE11 🙃 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer/isView

[JamieMason]

what would be the most appropriate file to put a test in for this? test/browser/browser.spec.js, one of test/node/*.spec.js, or create a new file for testing host environment conditions?

[chrisbolin]

@JamieMason this is still an issue with IE11 right? i'll keep it open if it is

[JamieMason]

Hi @chrisbolin, this is still present yes – have reopened.

[JamieMason]

On ArrayBuffer.isView being supported in IE11 – another theory might be a bad Polyfill somewhere in the Stack, or one released in the period between ArrayBuffer and ArrayBuffer.isView being adopted – I don't know. The environment is outside of our control but we can take small measures to defend ourselves against it.

EDIT:

1. I have a proposed fix here you can see, I can raise a PR and let me know about IE 11: Object doesn't support property or method 'isView' #66 (comment) if you want tests: https://github.com/FormidableLabs/react-fast-compare/compare/master...JamieMason:issue/66?expand=1

2. Totally off-topic, but I played around with some optimisations in this branch https://github.com/JamieMason/react-fast-compare/pull/new/optimisations and measured consistent improvements, it spoils the boundary you've been trying to maintain via comments between this and your upstream library though, but it's there incase you're interested in the ideas.

   Benchmarks

   MASTER BRANCH
   
   --- speed tests: generic usage ---
   
   react-fast-compare x 167,026 ops/sec ±3.17% (86 runs sampled)
   react-fast-compare x 170,456 ops/sec ±1.92% (89 runs sampled)
   react-fast-compare x 165,394 ops/sec ±2.68% (90 runs sampled)
   
   --- speed tests: generic and react ---
   
   react-fast-compare x 79,449 ops/sec ±3.06% (86 runs sampled)
   react-fast-compare x 80,047 ops/sec ±4.81% (89 runs sampled)
   react-fast-compare x 86,828 ops/sec ±0.91% (93 runs sampled)
   
   OPTIMISATIONS BRANCH (commit 65ca248)
   
   --- speed tests: generic usage ---
   
   react-fast-compare x 182,872 ops/sec ±0.95% (94 runs sampled)
   react-fast-compare x 178,770 ops/sec ±0.95% (90 runs sampled)
   react-fast-compare x 172,519 ops/sec ±2.54% (89 runs sampled)
   
   --- speed tests: generic and react ---
   
   react-fast-compare x 85,501 ops/sec ±3.29% (87 runs sampled)
   react-fast-compare x 87,158 ops/sec ±2.29% (89 runs sampled)
   react-fast-compare x 91,644 ops/sec ±0.60% (92 runs sampled)
   
   OPTIMISATIONS BRANCH (commit 1f878f3)
   
   --- speed tests: generic usage ---
   
   react-fast-compare x 179,164 ops/sec ±1.41% (87 runs sampled)
   react-fast-compare x 175,094 ops/sec ±2.73% (86 runs sampled)
   react-fast-compare x 171,544 ops/sec ±2.81% (83 runs sampled)
   
   --- speed tests: generic and react ---
   
   react-fast-compare x 91,610 ops/sec ±0.69% (92 runs sampled)
   react-fast-compare x 90,979 ops/sec ±0.50% (94 runs sampled)
   react-fast-compare x 85,835 ops/sec ±3.34% (89 runs sampled)
   
   

[ryan-roemer]

@JamieMason -- Is there perhaps a simple reproduction (no minification, etc.) you can host online / in a public repository that we can then open in an ie11 VM and confirm?

[JamieMason]

A reduced test case can be helpful to isolate a problem when there's a lot of noise and can be a really good idea, but could you help me understand why one would be needed in this case @ryan-roemer? Let me explain my thinking – the only variable I can think of is whether IE11 supports ArrayBuffer.isView or not, and our VM could assert that via its Developer Console.

We can also see how this library would behave under the condition that ArrayBuffer is present but ArrayBuffer.isView is not by reading the source.

So I think the only question left is whether we feel that, on balance, it's worth doing. It makes an uncaught exception impossible, but in old and dying browsers, and adds code that now has to be maintained.

[ryan-roemer]

So, when I do just that test in IE11 vm via https://www.browserling.com/ it's all there:

[ryan-roemer]

So I think if we think the behavior you're seeing is a bug in react-fast-compare, I'd love to see a failing test case that uses react-fast-compare and throws on ie11...

[JamieMason]

I'm totally happy to close it Ryan

[chrisbolin]

it makes sense that it's tough to repro, as the original error came from Sentry—so not even a machine you controlled

[JamieMason]

So whether it happens in specifically IE11 is a Red Herring, whether it does or doesn't we still know that Browsers exist with this level of intermediate support and, just like IE 11, they're way out on the fringes.

the only question left is whether we feel that, on balance, it's worth doing. It makes an uncaught exception impossible, but in old and dying browsers, and adds code that now has to be maintained.

It's such low priority you could live with it and be fine and I honestly don't mind either way whether you implement a fix or not, I'm not invested and have no reason to sell you it – I saw the error in Sentry and thought it helpful to bring it to your attention.

I think you have all the information you need so I'll end my involvement here. Thanks for your time and for this great Project 👏

[antonjb]

Hey team,

This has been popping up in our error logs recently, which is how I stumbled across this issue. At least in our codebase it appears via react-helmet-async. The errors are all IE11 (except for two IE Mobile 11).

[ryan-roemer]

Any chance you have a simple reproduction that uses the library that errors that we can try in ie11?

[kitten]

This is probably quite out there, but according to this from 2016 there are older versions of IE 11 that don’t support it... mapbox/pbf#72
I don’t really know what to say. IE 11 is one thing, but... old versions of IE 11 😆

[JamieMason]

Here you go Ryan: https://jamiemason.github.io/react-fast-compare/

[chrisbolin]

Thanks @JamieMason and @antonjb! We should have a patch out soon.

[ryan-roemer]

I'm still not getting the reproduction on ie11, so maybe it's an old ie11 thing or whatnot as mentioned above.

At any rate, here's a suggestion for us for a guard at top:

var hasArrayBuffer = typeof ArrayBuffer === 'function' && typeof ArrayBuffer.isView === 'function';

or a slightly more terse version that just uses prop existence instead of function test:

var hasArrayBuffer = typeof ArrayBuffer === 'function' && !!ArrayBuffer.isView;

[JamieMason]

Thanks, there is a branch ready to put forward as a PR in #66 (comment) incase that would be of any use.

[ryan-roemer]

Thanks @JamieMason! -- we're super concerned with overall gzip size and the "terse version" I have above I'm pretty sure will give us smallest overall size (and avoids unneeded functions calls vs. an extra noop function call).

Thanks too for the optimizations branch, but I'm guessing they will actually get bigger for overall gzip size as gzip usually does better without aliases like var ObjectPrototype = Object.prototype;, etc. You can check out with yarn size-min-gz in dev vs. current master.

[JamieMason]

Thanks a lot @ryan-roemer. On the fix branch that is a good point about gzip-suitability, I hadn't factored that in.

To add some context on the optimizations branch I poked around with – the changes there were only around raw compute speed, rather than helping a minifier or gzip reduce the file size – so those changes like var ObjectPrototype = Object.prototype; improved performance (negligibly) by caching the result of getting Object then reading prototype from it every time. This caching optimization is really small, but appears in a recursive code path – so I guess it could add up when comparing larger datasets.

I added some Benchmarks in a collapsed <details> element in #66 (comment). The tl;dr is a small improvement, these are the numbers from your "general and react" suite.

branch	worst ops/sec	best ops/sec
master	79,449	86,828
optimizations	85,835	91,610

I hadn't measured the filesize but I imagine it would likely gain a little weight.

[antonjb]

I'm still not getting the reproduction on ie11, so maybe it's an old ie11 thing or whatnot as mentioned above.

I couldn't get it either. Had a look at the user agent string of one of the issues in our error tracking. For interest:
Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; rv:11.0) like Gecko

Operating system: Windows 7

Tried using a simulated browser and Jamie's example...and still don't get it.

[kale-stew]

Hi @JamieMason @antonjb - we just released this fix in v3.0.2. Can you upgrade and let us know if you're still seeing this err? Thanks again for bringing this to our attention.

[JamieMason]

Will do, thanks @kale-stew

[antonjb]

That's awesome, thanks for addressing the issue team. React Fast Compare arrives to our codebase as a dependency so I'll need to head to those repos and see if it can be updated.