[v4] [icons] fix: pre-es2017 compatibility #5098
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5097
Changes proposed in this pull request:
Switches an
Object.values
usage to afor ... in ...
loop for pre-es2017 browser compatibility.Reviewers should focus on:
Edit: The
guard-for-in
ESLint rule triggered on the below. Updated the commit forpropertyIsEnumerable
.According to MDN,
Object.values
also filters on own enumerable properties, so technically we should also check forObject.prototype.propertyIsEnumerable()
.TypeScript doesn't seem to generate non-own enumerable properties for enum objects though, so in practice the extra check isn't needed. https://www.typescriptlang.org/play?#code/KYOwrgtgBAYg9nKBvAsAKCpqAhAhgJygF4oAiAIwNPQF910AzOQgCgGM4QBnAFygGtgATygBLELAQBKZOixQO3OABtgAOmVwA5i0FCptIA
Happy to add it if reviewers think the extra soundness is worthwhile.