Skip to content

Conversation

@seanparmelee
Copy link
Contributor

  • This is a bugfix
  • This is a new feature
  • This is a code refactor
  • This is a test update
  • This is a typo fix
  • This is a metadata update

For Bugs and Features; did you add new tests?

No

Motivation / Use-Case

To maintain IE 10 & 11 compatibility, Symbol needs to be polyfilled since these browsers do not support it. Fixes #5.

Breaking Changes

None

Additional Info

This PR builds off the commit in your ie-assign-symbol branch; not sure if you wanted those changes regardless but if not I can remove them. With this PR, the loglevelnext.min.js file bumps up to 18 kB. I also gave useBuiltIns: 'usage' a shot and that bumped the size to 30 kB.

@codecov
Copy link

codecov bot commented Apr 25, 2018

Codecov Report

Merging #6 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage    93.7%   93.79%   +0.08%     
==========================================
  Files           4        4              
  Lines         143      145       +2     
  Branches       24       24              
==========================================
+ Hits          134      136       +2     
  Misses          9        9
Impacted Files Coverage Δ
lib/MethodFactory.js 86.79% <100%> (ø) ⬆️
index.js 96.77% <100%> (+0.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a7ab43...8309daa. Read the comment docs.

"presets": [
["@babel/preset-env", {
"targets": {
"browsers": ["last 2 versions", "IE 10"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change was already in the branch. not sure if you prefer to keep it, but fwiw: https://jamie.build/last-2-versions

}

for (const methodName of this.methods) {
this.methods.forEach((methodName) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The polyfill didn't place nicely with the Babel transformation code that results from the for..of so I changed this to a forEach. IE 11 was thowing an Object expected error.

@shellscape
Copy link
Owner

Thanks for putting that together. I'll take a look at it later tonight.

@MatTheCat
Copy link

MatTheCat commented Apr 26, 2018

Using this branch I no longer have an error regarding Symbol. There is another problem affecting IE and Edge but it seems to be fixed by webpack-contrib/webpack-hot-client#43

@shellscape shellscape merged commit e82e379 into shellscape:master Apr 26, 2018
@shellscape
Copy link
Owner

Thanks for your work on this 🍺

In the future please keep from adding dist files and package-lock.

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.

Internet Explorer 11 compatibility?

3 participants