lz-string icon indicating copy to clipboard operation
lz-string copied to clipboard

Replace objects with Maps to ensure compatibility with Hermes

Open danii1 opened this issue 1 year ago • 4 comments

This PR replaces objects with Map to alleviate the issue with Hermes runtime(React Native) that doesn't support a large number of props in an object. This allows to use this package in React Native apps.

Related issue: https://github.com/pieroxy/lz-string/issues/168

danii1 avatar Feb 21 '24 05:02 danii1

This needs to compare with #98 and have a look at any side effects etc mentioned - not got time to review myself now, but of the general opinion that it's worth adding :-)

Rycochet avatar Feb 21 '24 16:02 Rycochet

I have another branch with the same changes applied to v2: https://github.com/DISCO-team/lz-string/tree/feature/object-map

Ran test:bench, Map seems faster in most scenarios(particularly where it matters like compressing large text), not surprising given it's optimized for addition and deletion: obj.txt map.txt

Changes in this PR are straightforward and simpler than in #98

danii1 avatar Feb 21 '24 18:02 danii1

@Rycochet @danii1 Any further progress likely here? The changes look trivial with a cursory glance :)

adbs1 avatar May 09 '24 10:05 adbs1

The change is solid and has my vote of support from a code standpoint, but needs a final decision from a compatibility angle. LZ-String has been maintaining compatibility with anything javascript released since roughly 2012, and this change will break that trend. I believe there is a limit to backwards compatibility and a move to v2 is a good time to revisit that support, but is ultimately not my decision to make. @Rycochet @pieroxy what are your thoughts?

karnthis avatar May 30 '24 20:05 karnthis