Level / level-js

An abstract-leveldown compliant store on top of IndexedDB.
MIT License
544 stars 44 forks source link

Avoid instanceof (for cross-realm support) #82

Closed vweevers closed 6 years ago

vweevers commented 6 years ago

Rewrite value instanceof ArrayBuffer etc.

vweevers commented 6 years ago

Only one left actually:

https://github.com/Level/level-js/blob/3a8b861b550ef18fd0b7055e5eb916a0a1f8c33a/index.js#L128

The other one doesn't matter because it's only used on values coming out the IndexedDB:

https://github.com/Level/level-js/blob/3a8b861b550ef18fd0b7055e5eb916a0a1f8c33a/util/mixed-to-buffer.js#L6

ralphtheninja commented 6 years ago

What do you mean by cross-realm support and why is instanceof problematic?

vweevers commented 6 years ago

See https://jakearchibald.com/2017/arrays-symbols-realms/#multiple-realms

vweevers commented 6 years ago

Also https://github.com/jasnell/proposal-istypes

vweevers commented 6 years ago

And https://github.com/ljharb?utf8=%E2%9C%93&tab=repositories&q=is-&type=&language=

ralphtheninja commented 6 years ago

So, yeah, feeling pretty hipster right now.

:laughing:

ralphtheninja commented 6 years ago

@vweevers Are there any rules on when it is important and not important, e.g. why isn't instanceof in the constructor and the Uint8Array important?

vweevers commented 6 years ago

It depends on the likelihood that the value was created in a different realm. For keys and values coming straight out of IndexedDB it doesn't matter - as we know they come from the current realm (where IndexedDB and the structured clone algorithm lives).

It pretty much only matters for user input. When a user does db.get(date), that date key might have come from a different realm.