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

Remove polyfills #3040

Merged
merged 3 commits into from
Nov 3, 2017
Merged

Remove polyfills #3040

merged 3 commits into from
Nov 3, 2017

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Nov 3, 2017

  • Removes Map and setImmediate polyfills.

BREAKING CHANGE: groupBy requires a Map polyfill in older runtimes
BREAKING CHANGE: asap scheduling requires a Promise polyfill in older runtimes

- Removes Map polyfill
- Removes FastMap impl - Tests in latest Chrome and Node show no discernable difference in performance between Map<string, T> and Object has a hashtable for our use case.
- Adds an error that is thrown immediately at runtime if Map does not exist.

BREAKING CHANGE: Older runtimes will require Map to be polyfilled to use
`groupBy`
Gets rid of fancy polyfill and uses Promise to schedule instead, since Promise is required for several key parts of the library to work. Also setImmediate does not appear to be getting standardized.

BREAKING CHANGE: Old runtimes must polyfill Promise in order to use ASAP scheduling.
BREAKING CHANGE: Using `distinct` requires a `Set` implementation and must be polyfilled in older runtimes
@rxjs-bot
Copy link

rxjs-bot commented Nov 3, 2017

Warnings
⚠️

❗ Big PR (1)

Messages
📖

(1) : Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.

CJS: 1377.0KB, global: 745.2KB (gzipped: 118.5KB), min: 146.0KB (gzipped: 31.5KB)

Generated by 🚫 dangerJS

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 97.223% when pulling 68ee499 on benlesh:remove_polyfills into 0f3cf71 on ReactiveX:master.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 3, 2017

Coverage Status

Coverage decreased (-0.04%) to 97.223% when pulling 68ee499 on benlesh:remove_polyfills into 0f3cf71 on ReactiveX:master.

Copy link
Contributor

@rgbkrk rgbkrk left a comment

Choose a reason for hiding this comment

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

This is fantastic!

@kwonoj kwonoj merged commit cff9dfa into ReactiveX:master Nov 3, 2017
@kwonoj kwonoj mentioned this pull request Feb 21, 2018
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants