feat!(NODE-4712): remove unused Map polyfill

[nbbeeken]

Description

What is changing?

Removes unused Map implementation, and removes the export.
Adds an entry point test modeled after the one we have in our driver: here

Is there new documentation needed for these changes?

Migration guide updated.

What is the motivation for this change?

ES Map is available on all supported runtimes.

Double check the following

• Ran npm run lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[baileympearson]

Can you explain further why this is necessary? I don't understand why isWeb is added for our tests

[nbbeeken]

It is to identify when the export was created in an environment without Buffer (web-like, only TextEncoder/Decoder and btoa/atob globals). So we can change assertions based on that condition: for example, in the error_test.js file we can check instanceof on Node and .name prop in the web.

[baileympearson]

How thoroughly do we want to test this functionality? The tests cases seem random (a map with one key, nested map and numbered keys?). I'd argue that if, for the purposes of this PR, we only care that we still support maps, then it should be sufficient to just serialize an empty map and confirm 1) there's no error thrown and 2) it serializes as a document type. If we want to add testing to our map serialization, then these test cases feel insufficient and random.

Also, the nested document test feels random. If we test nested maps, shouldn't we test all valid nesting scenarios we can think of? A map nested in an array or an array in a map in an array, ect? My feeling is that it's probably unnecessary to test these nested scenarios at all, but I'm curious to hear others' opinions.

Lastly, the test for ordering of keys feels insufficient. If we test that case, shouldn't we test that insertion order is respected for the following cases (and maybe more, if we can think of more):

• numeric keys
• string keys
• numeric and string keys
• string keys that are numeric (i.e., "1", "2")
• numeric keys and string keys that are numeric

---

I'm curious what @dariakp thinks too

[dariakp]

Well, we should determine what the purpose of the tests is (i.e., what is important for this functionality to do); if it's a sanity check, then an empty map and a non-empty map would be sufficient (empty is always an edge case, so I wouldn't advise testing solely with empty input). We should also look at the removed tests and determine if any of those are worth preserving (e.g., the object that looks like a map).

To help determine the purpose, we can see what it is we are actually implementing on top of what the built in construct already does and look for any branching conditions. I can't give more concrete guidance without digging through the code, hopefully @nbbeeken has enough context to answer these questions.

[nbbeeken]

A lot more thoroughly than we currently do, we have some smattering of Map tests already in the bson_test.js file, my goal was to bring us some organized sanity checks using our more modern testing practices. So we don't need new tests at all since we do have the coverage over there. I kept them simple to make the bytewise assertions reasonable, but they are arbitrary, including the nested Map test, it was only inspired by being an "interesting" case of using Maps.

What we really need to file a ticket for is fuzz testing of some kind that is much more structured about generating arbitrarily shaped objects/maps/arrays with all various types, which I can do! (done)

The key ordering test is specifically trying to prove the use case we've relied on Maps for (the ability to keep numeric keys in chronological order) within the driver code (sorts and index definitions). This test provides an early warning if that were to change as we work more on the BSON library. So yes we should test insertion order for all those things, with some knowledge that there's diminishing return since we'd be testing ES Map behavior. The stringified numeric keys is an "interesting" case worth keeping here since it is so easy to break, as soon as the keys from the Map land in an object we've lost the ordering information. (ex. Object.fromEntries(map) seemingly harmless line of code actually can have a big impact on BSON output)

[baileympearson]

I was thinking along the lines of what Daria said - that just simple cases would suffice for this PR (thanks for adding the empty document test case). I'm okay with extra testing too.

Also - thanks for adding the bson fuzzing ticket

[baileympearson]

Suggested change

	This library no longer polyfills [ES Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) and the export "Map" was removed. Users should migrate to using the global Map constructor available in all supported JS environments.
	This library no longer polyfills [ES Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) and the export "Map" was removed. If users are instantiating the exported `Map` object from the BSON library instead of the global map object, users should migrate to using the global Map constructor available in all supported JS environments.

optional