Skip to content
This repository has been archived by the owner on Feb 11, 2021. It is now read-only.

Replace PointerMap with polyfill for regular Map #189

Closed
wants to merge 1 commit into from

Conversation

stuartpb
Copy link
Contributor

Ideally, this polyfill would be left out of this project altogether (to be handled by a dedicated polyfill like CoreJS), but that can be done later.

@stuartpb
Copy link
Contributor Author

Just signed the CLA, not sure why the status hasn't updated.

@scottgonzalez
Copy link
Contributor

I'm not sure what this actually gets us. We're still not using the standard Map API even with this update.

@scottgonzalez
Copy link
Contributor

In fact, won't this just throw errors in any browser that has Map?

@stuartpb
Copy link
Contributor Author

This isn't using the standard Map API? What's different?

@scottgonzalez
Copy link
Contributor

Map#size is not a function.

@stuartpb
Copy link
Contributor Author

Right - so the polyfill should implement a getter for size instead of a function. On that now.

@stuartpb stuartpb force-pushed the just-map branch 2 times, most recently from 7b87104 to b8578a5 Compare May 18, 2015 22:13
@stuartpb
Copy link
Contributor Author

size uses a getter now.

@stuartpb
Copy link
Contributor Author

Fixed tests (I think, I'm not familiar with intern).

@stuartpb stuartpb force-pushed the just-map branch 3 times, most recently from 1349161 to 46b3172 Compare May 18, 2015 22:43
@stuartpb
Copy link
Contributor Author

Derp, changing how existing Map() is looked up now.

@stuartpb
Copy link
Contributor Author

@stuartpb
Copy link
Contributor Author

Okay, the only errors in the build now are the ones that are present in the current master.

@stuartpb
Copy link
Contributor Author

So, since this is only needed to map pointers with (low) integer IDs for keys, why not just use a sparse Array to polyfill the Map?

@scottgonzalez
Copy link
Contributor

We went with the sparse array implementation from gh-190.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants