Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of Map and WeakMap #143

Open
oscar-b opened this issue Oct 26, 2016 · 6 comments
Open

Use of Map and WeakMap #143

oscar-b opened this issue Oct 26, 2016 · 6 comments

Comments

@oscar-b
Copy link

oscar-b commented Oct 26, 2016

Is the use of Map and WeakMap necessary? I recently added error tracking to our site and found out that old clients barf on these requirements. The alternative is of course to polyfill, but that should probably be documented.

@qwales1
Copy link
Collaborator

qwales1 commented Oct 26, 2016

@oscar-b I don't think Map is necessary for keeping track of listeners and refs. Could use a different approach. The WeakMap is used because React does not have a unique identifier for each component and syncState needs that. Which clients are having issues?

@oscar-b
Copy link
Author

oscar-b commented Oct 26, 2016

@qwales1 Older versions of Chrome (v34) used in Samsung phones from 2014 branded as Samsung Browser.

@qwales1
Copy link
Collaborator

qwales1 commented Oct 26, 2016

@oscar-b does the error reporting tell you what specifically its blowing up on? Is it Symbol?

@oscar-b
Copy link
Author

oscar-b commented Oct 26, 2016

ReferenceError
Map is not defined

I guess it's: new WeakMap? Or what do you mean?

@qwales1
Copy link
Collaborator

qwales1 commented Oct 26, 2016

@oscar-b Yes sorry that is what I was asking. Could add es6-map and es6-weak-map ponyfills as dependencies. They do not mess with the global scope.

@oscar-b
Copy link
Author

oscar-b commented Oct 27, 2016

I would either refactor to use regular object/array or add documentation about compatibility and use of something like polyfill.io.

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

Successfully merging a pull request may close this issue.

2 participants