Maps are broken

[mweststrate]

Just as a quick check, I took some of the old tests of the earlier closest PR to add map support (#149), and they all seem to fail, although the look superficially speaking correct to me. Am I missing something obvious?

cc @aleclarson @keenondrums

[aleclarson]

The tests should be wrapped with if (useProxies)

Once that was added, only two of these tests fail

[mweststrate]

ES5 is not supported? Then we should fix that first before releasing this to the public. Shall we unpublish 3.1.0 for now?

…

On Thu, Apr 18, 2019 at 3:43 PM Alec Larson ***@***.***> wrote: The tests should be wrapped with if (useProxies) Once that was added, only two of these tests fail — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBFRFIRENEDG5URFBT3PRB3I3ANCNFSM4HG47CSA> .

[aleclarson]

@mweststrate I see ES5 support as an enhancement more than a requirement. Users should still avoid Map objects if they need to target ES5. I can add a warning in the readme.

[mweststrate]

No, that is a no-go area; Immer does support and maps can be polyfilled no problem for IE 11. Proxies are a different ball game. The result of this change is that devs can work happily for months on there immer + map projects while devving. Having it blown into pieces the first time a IE using customer arrives... Maps should be supported in the ES5 implementation as well, or not be released to avoid a lot of confusion and angry bug reports.

…

On Thu, Apr 18, 2019 at 3:51 PM Alec Larson ***@***.***> wrote: @mweststrate <https://github.com/mweststrate> I see ES5 support as an enhancement more than a requirement. Users should still avoid Map objects if they need to target ES5. I can add a warning in the readme. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBADHXT7QP47ZYPS5DTPRB4F7ANCNFSM4HG47CSA> .

[mweststrate]

Also note that adding map support is a breaking change, as having maps in a draft is going to behave entirely different now :). Unpublishing and reverting the MR for now before someone trips over it. On Thu, Apr 18, 2019 at 3:57 PM Michel Weststrate <mweststrate@gmail.com> wrote:

…

No, that is a no-go area; Immer does support and maps can be polyfilled no problem for IE 11. Proxies are a different ball game. The result of this change is that devs can work happily for months on there immer + map projects while devving. Having it blown into pieces the first time a IE using customer arrives... Maps should be supported in the ES5 implementation as well, or not be released to avoid a lot of confusion and angry bug reports. On Thu, Apr 18, 2019 at 3:51 PM Alec Larson ***@***.***> wrote: > @mweststrate <https://github.com/mweststrate> I see ES5 support as an > enhancement more than a requirement. Users should still avoid Map > objects if they need to target ES5. I can add a warning in the readme. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#353 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AAN4NBADHXT7QP47ZYPS5DTPRB4F7ANCNFSM4HG47CSA> > . >

[aleclarson]

Whoops, sorry about that! 😅

[aleclarson]

Also forgot to update the TypeScript defs :S

[mweststrate]

Could you revert and open a new PR? Let's take such a fundamental change a bit slower next time :)

…

On Thu, Apr 18, 2019 at 4:03 PM Alec Larson ***@***.***> wrote: I made a whoopsie! :P — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBDI2O3XPM2GY4SUZQLPRB5U7ANCNFSM4HG47CSA> .

[aleclarson]

#354