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

Update polyfills to use core-js. #1142

Merged
merged 1 commit into from
Apr 20, 2017
Merged

Conversation

nanek
Copy link
Contributor

@nanek nanek commented Apr 20, 2017

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

Reduces file size by 9K. Fixes #1125. From https://babeljs.io/docs/usage/polyfill/ which recommends https://github.com/zloirock/core-js#commonjs

I'd prefer to revert #962 over this to save an additional 6K, as I can't reproduce the IE error. I think they were basically the standard polyfill from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find

When do you run IE tests? Is travis-ci doing this on each PR? Is there coverage for this?

@protonate
Copy link
Collaborator

We currently run IE tests during release prep but are working on setting up Travis-ci and Browserstack integration for builds.

The polyfills used previously are from MDN.

As it happens I'm testing IE now and we're also breaking on for ... of loops as Babel uses Symbol.iterator.

Would you update this PR to require('core-js/fn/symbol/iterator')?

@mkendall07
Copy link
Member

We've had some nagging issues with unit tests not playing nicely in all IE versions so we've disabled IE tests with Travis-ci for now. but we want to get them added and working soon.

@nanek
Copy link
Contributor Author

nanek commented Apr 20, 2017

require('core-js/fn/symbol/iterator') adds another 4K. Can we use forEach instead of polyfilling for...of?

I could only find one use of it:

for (let key of Object.keys(params)) {

@mkendall07 mkendall07 self-requested a review April 20, 2017 15:24
@@ -828,7 +828,7 @@ babel-polyfill@^6.13.0:
core-js "^2.4.0"
regenerator-runtime "^0.10.0"

babel-preset-es2015@^6.5.0:
Copy link
Member

Choose a reason for hiding this comment

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

can you revert these changes? I don't think they relate to the polyfill

Copy link
Collaborator

@protonate protonate left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, we'll look further at Symbol.iterator and for...of issue.

@protonate protonate merged commit 4b9b2a9 into prebid:master Apr 20, 2017
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Apr 26, 2017
…built

* 'master' of https://github.com/prebid/Prebid.js: (23 commits)
  Prebid 0.22.2 Release
  Renderer test - use httpbin/post endpoint in place of domain alone
  AdKernal - comment out console.warn in test
  WideOrbit - remove test referencing url with redirect
  add karma-es5-shim again via yarn
  Widespace - fix lint error on build and test
  Fix Object.assign support for IE11 & <
  Widespace - use forEach in place of for...of
  Underdogmedia - use forEach in place of for...of
  PlusepointLite - comment out breaking tests
  Prebid 0.22.1 Release
  set babel plugins @6.22.0
  lock down babel packages @6.24.1
  PulsePointLite: comment out breaking adapter test
  Prebid 0.22.0 Release
  Update polyfills to use core-js. (prebid#1142)
  Mantis - comment out breaking test
  Add Trion Interactive adapter (prebid#1059)
  HuddledMasses header bidding adapter (prebid#1095)
  Add support for custom floor value in rubicon video and increased code coverage. (prebid#1102)
  ...
vzhukovsky added a commit to aol/Prebid.js that referenced this pull request May 22, 2017
…20.0 to aolgithub-master

* commit '80a37975d00dadcb5b3e995e26b6fe0a0435f873': (42 commits)
  Updated build status indicator.
  Added adapters in aolPartnersIds.json.
  Added changelog entry.
  Prebid 0.22.2 Release
  Renderer test - use httpbin/post endpoint in place of domain alone
  AdKernal - comment out console.warn in test
  WideOrbit - remove test referencing url with redirect
  add karma-es5-shim again via yarn
  Widespace - fix lint error on build and test
  Fix Object.assign support for IE11 & <
  Widespace - use forEach in place of for...of
  Underdogmedia - use forEach in place of for...of
  PlusepointLite - comment out breaking tests
  Prebid 0.22.1 Release
  set babel plugins @6.22.0
  lock down babel packages @6.24.1
  PulsePointLite: comment out breaking adapter test
  Prebid 0.22.0 Release
  Update polyfills to use core-js. (prebid#1142)
  Mantis - comment out breaking test
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants