Skip to content

[themeing] Use Object.create(null) instead of {} for TokenRegistry#103349

Merged
aeschli merged 2 commits intomicrosoft:masterfrom
nrayburn-tech:semanticobjects
Aug 11, 2020
Merged

[themeing] Use Object.create(null) instead of {} for TokenRegistry#103349
aeschli merged 2 commits intomicrosoft:masterfrom
nrayburn-tech:semanticobjects

Conversation

@nrayburn-tech
Copy link
Copy Markdown
Contributor

Replaces {} with Object.create(null), so that accessing the object can have properties defined that conflict with the prototype names. constructor, toString are examples.

This PR fixes #103104

@aeschli
Copy link
Copy Markdown
Contributor

aeschli commented Aug 11, 2020

Thanks @nrayburn-tech !

@aeschli aeschli added this to the August 2020 milestone Aug 11, 2020
@aeschli aeschli merged commit e42f5a6 into microsoft:master Aug 11, 2020
@aeschli aeschli added bug Issue identified by VS Code Team member as probable bug editor-theming Issues related to the way themes are applied to editors labels Aug 11, 2020
@aeschli aeschli changed the title Use Object.create(null) instead of {} for TokenRegistry [themeing] Use Object.create(null) instead of {} for TokenRegistry Aug 11, 2020
@jrieken jrieken added the verified Verification succeeded label Sep 2, 2020
@jrieken
Copy link
Copy Markdown
Member

jrieken commented Sep 2, 2020

Maybe it's time for native Map?

@nrayburn-tech
Copy link
Copy Markdown
Contributor Author

@jrieken where all would Map be preferred? Anywhere that Object.create(null) is used?

There are 180ish files that use Object.create(null). I can work on a PR to get the simple uses converted, but is there a preferred way to submit it, so it isn't so overwhelming to review?

Should I create an issue for this?

@jrieken
Copy link
Copy Markdown
Member

jrieken commented Sep 2, 2020

Should I create an issue for this?

No please :-) We generally don't accept PRs that go across different ownerships and make changes like this. Tho, when revisiting "local" things, as it has been done here, it is always a good to consider to use modern JS

@aeschli
Copy link
Copy Markdown
Contributor

aeschli commented Sep 3, 2020

Thanks, no PR needed, I prefer object literals.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Issue identified by VS Code Team member as probable bug editor-theming Issues related to the way themes are applied to editors verified Verification succeeded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Semantic token colorization fails when a token type ID matching any object.prototype property name is provided

4 participants