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

fix channel names as object prototype keys #777

Merged
merged 10 commits into from
Jul 30, 2021
Merged

Conversation

Matt-Esch
Copy link
Contributor

When an object key is free-form text, we should not use vanilla js objects, as this can cause key collisions with built-in prototype keys, such as __proto__ or toString, etc. I have offered a quick solution for this issue, but it would be better tackled using an ES6 Map. I am happy for someone to replace these structures with Map instances if they would prefer. I'm not familiar with which versions of js we support and whether we would want a Map polyfill if things are being back-ported (babel?) for < ES6 compatibility.

@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 26, 2021 12:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 27, 2021 10:45 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 28, 2021 02:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 28, 2021 02:41 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 28, 2021 17:12 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/777/bundle-report July 28, 2021 19:00 Inactive
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing the presence test as well 👍

@Matt-Esch
Copy link
Contributor Author

I think I have satisfied myself that these tests are unaffected by the changes I've made so please go ahead and merge this for me please @owenpearson - I have fixed a few of the tests, I got to the point of debugging firefox tests and that's a bit too deep in the weeds at this point. I have at least seen passing tests for node 10/12/14 so if you're happy with this let's get it merged.

@owenpearson owenpearson merged commit 30b7cdd into main Jul 30, 2021
@owenpearson owenpearson deleted the me-fix-object-channel-id branch July 30, 2021 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants