Comments (26)
Here you go Ryan: https://jamiemason.github.io/react-fast-compare/
from react-fast-compare.
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 😆
from react-fast-compare.
Thanks @JamieMason and @antonjb! We should have a patch out soon.
from react-fast-compare.
Thanks, there is a branch ready to put forward as a PR in #66 (comment) incase that would be of any use.
from react-fast-compare.
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
from react-fast-compare.
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
from react-fast-compare.
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?
from react-fast-compare.
@JamieMason this is still an issue with IE11 right? i'll keep it open if it is
from react-fast-compare.
Hi @chrisbolin, this is still present yes – have reopened.
from react-fast-compare.
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:
-
I have a proposed fix here you can see, I can raise a PR and let me know about #66 (comment) if you want tests: https://github.com/FormidableLabs/react-fast-compare/compare/master...JamieMason:issue/66?expand=1
-
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)
from react-fast-compare.
@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?
from react-fast-compare.
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.
from react-fast-compare.
So, when I do just that test in IE11 vm via https://www.browserling.com/ it's all there:
from react-fast-compare.
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...
from react-fast-compare.
I'm totally happy to close it Ryan
from react-fast-compare.
it makes sense that it's tough to repro, as the original error came from Sentry—so not even a machine you controlled
from react-fast-compare.
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 👏
from react-fast-compare.
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).
from react-fast-compare.
Any chance you have a simple reproduction that uses the library that errors that we can try in ie11?
from react-fast-compare.
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;
from react-fast-compare.
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.
from react-fast-compare.
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.
from react-fast-compare.
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.
from react-fast-compare.
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.
from react-fast-compare.
Will do, thanks @kale-stew
from react-fast-compare.
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.
from react-fast-compare.
Related Issues (20)
- Upgrade dependencies
- Add ability to compare functions HOT 9
- module '/node_modules/react-fast-compare/index.js' does not provide an export HOT 1
- Handle anonymous function HOT 2
- Bug: Set compares by reference not value HOT 6
- TypeScript library for react-fast-compare? HOT 9
- Fix exported types so that they work for react-redux HOT 8
- Cannot compare object will null prototype HOT 11
- Getters are not considered HOT 2
- There should be a 'debug' feature to determine which fields are breaking equality.
- Types: documentation, clean up, and testing
- Audit security alerts HOT 2
- Fix benchmark chart img path
- Ignore Arrow Functions HOT 1
- Re-evaluate project's eslint rules HOT 1
- Update benchmark png HOT 2
- CJS + ESM Question/discussion HOT 6
- Differences in objects with properties that are associative arrays are not detected HOT 8
- Infra: Switch to GitHub Actions HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-fast-compare.