Add ES2024 and ES2025 polyfills for Safari 17.0 compatibility#104
Closed
Add ES2024 and ES2025 polyfills for Safari 17.0 compatibility#104
Conversation
Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
- Add Object.groupBy polyfill (ES2024) - Add Map.groupBy polyfill (ES2024) - Add Iterator helpers polyfill (ES2025): map, filter, take, drop, flatMap, reduce, toArray, forEach, some, every, find - Add Iterator.from polyfill (ES2025) - Add Promise.try polyfill (ES2025) - Add comprehensive tests for all new polyfills - Update src/index.ts to register new polyfills - All tests passing (56 tests) Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds polyfills for ES2024 and ES2025 features to support Safari 17.0 browser compatibility. The implementation includes Object.groupBy, Map.groupBy, Promise.try, and comprehensive Iterator helpers (11 methods plus Iterator.from).
Changes:
- Added ES2024 polyfills: Object.groupBy and Map.groupBy for grouping array elements by key
- Added ES2025 polyfills: Promise.try for wrapping function execution in promises, and Iterator helpers (map, filter, take, drop, flatMap, reduce, toArray, forEach, some, every, find) plus Iterator.from
- Integrated all new polyfills into src/index.ts following the established pattern
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/promise-try.ts | Implements Promise.try polyfill for wrapping synchronous/async function execution |
| src/object-groupby.ts | Implements Object.groupBy polyfill for grouping array elements into objects |
| src/map-groupby.ts | Implements Map.groupBy polyfill for grouping array elements into Maps |
| src/iterator-helpers.ts | Implements 11 Iterator prototype methods and Iterator.from static method |
| src/index.ts | Integrates new polyfills into the main polyfill registry |
| test/promise-try.js | Test coverage for Promise.try functionality |
| test/object-groupby.js | Test coverage for Object.groupBy functionality |
| test/map-groupby.js | Test coverage for Map.groupBy functionality |
| test/iterator-helpers.js | Comprehensive test coverage for all Iterator helpers and chaining |
| package-lock.json | Dependency lockfile updates (includes some unexpected peer dependency markings) |
Comments suppressed due to low confidence (4)
src/iterator-helpers.ts:60
- The iterator objects returned by filter don't inherit from Iterator.prototype, which will break method chaining. The returned objects should be created with Object.create(getIteratorPrototype() || {}) instead of plain object literals, or should have their prototype set to enable chaining.
return {
next() {
// eslint-disable-next-line no-constant-condition
while (true) {
const result = iterator.next()
if (result.done) {
return {done: true, value: undefined} as IteratorReturnResult<undefined>
}
if (predicate(result.value, index++)) {
return result
}
}
},
return(value?: T) {
if (iterator.return) {
iterator.return()
}
return {done: true, value} as IteratorReturnResult<T>
},
} as Iterator<T>
src/iterator-helpers.ts:85
- The iterator objects returned by take don't inherit from Iterator.prototype, which will break method chaining. The returned objects should be created with Object.create(getIteratorPrototype() || {}) instead of plain object literals, or should have their prototype set to enable chaining.
return {
next() {
if (remaining <= 0) {
if (iterator.return) {
iterator.return()
}
return {done: true, value: undefined} as IteratorReturnResult<undefined>
}
remaining--
return iterator.next()
},
return(value?: T) {
if (iterator.return) {
iterator.return()
}
return {done: true, value} as IteratorReturnResult<T>
},
} as Iterator<T>
src/iterator-helpers.ts:110
- The iterator objects returned by drop don't inherit from Iterator.prototype, which will break method chaining. The returned objects should be created with Object.create(getIteratorPrototype() || {}) instead of plain object literals, or should have their prototype set to enable chaining.
return {
next() {
while (remaining > 0) {
const result = iterator.next()
if (result.done) {
return {done: true, value: undefined} as IteratorReturnResult<undefined>
}
remaining--
}
return iterator.next()
},
return(value?: T) {
if (iterator.return) {
iterator.return()
}
return {done: true, value} as IteratorReturnResult<T>
},
} as Iterator<T>
src/iterator-helpers.ts:150
- The iterator objects returned by flatMap don't inherit from Iterator.prototype, which will break method chaining. The returned objects should be created with Object.create(getIteratorPrototype() || {}) instead of plain object literals, or should have their prototype set to enable chaining.
return {
next() {
// eslint-disable-next-line no-constant-condition
while (true) {
if (innerIterator) {
const result = innerIterator.next()
if (!result.done) {
return result
}
innerIterator = null
}
const result = iterator.next()
if (result.done) {
return {done: true, value: undefined} as IteratorReturnResult<undefined>
}
const mapped = mapper(result.value, index++)
innerIterator = Symbol.iterator in mapped ? mapped[Symbol.iterator]() : (mapped as Iterator<U>)
}
},
return(value?: U) {
if (innerIterator?.return) {
innerIterator.return()
}
if (iterator.return) {
iterator.return()
}
return {done: true, value} as IteratorReturnResult<U>
},
} as Iterator<U>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix test files to check isPolyfilled() === false (4 files) - Fix iterator helpers to inherit from Iterator.prototype for proper method chaining - Updated map, filter, take, drop, flatMap to use Object.create(iteratorPrototype) - Ensure globalThis.Iterator is created if it doesn't exist in apply() - All 56 tests still passing Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
6 tasks
Member
|
superceeded by #105 but not sure why copilot decided to do that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
isPolyfilled() === falsebefore polyfill is applied (4 test files)Original prompt