Skip to content

Remove ES6 code to fix old browser compatibility #209

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

Merged
merged 3 commits into from
Jun 29, 2018

Conversation

evanlok
Copy link
Contributor

@evanlok evanlok commented Jun 27, 2018

Number.isInteger is not supported on older browsers. babel polyfill fixes compatibility.

https://caniuse.com/#feat=es6-number

@dhilt
Copy link
Member

dhilt commented Jun 28, 2018

@evanlok Thanks a lot for the PR!

I'm investigating the difference. The main bundle size is being increased from 58 to 96 Kb. Replacing Number.isInteger with a simplest polyfill I'm getting 95 Kb with babel-plugin-transform-runtime plugin enabled. So what are the others 37 Kb? Do we really need babel-plugin-transform-runtime if Number.isInteger is the only problem?

@evanlok
Copy link
Contributor Author

evanlok commented Jun 28, 2018

@dhilt It looks like transform-runtime includes the entire polyfill and not just the ones used in the source code. I couldn't find any configuration that would change the behavior unfortunately. I've refactored this to implement isInteger function in a util module which should make the size increase negligible. I don't know if there is other ES6 code being used, but the changes I've made are working for me on IE11.

@evanlok evanlok changed the title Add babel-plugin-transform-runtime to fix old browser compatibility Remove ES6 code to fix old browser compatibility Jun 28, 2018
@dhilt
Copy link
Member

dhilt commented Jun 28, 2018

@evanlok oh, Angular already has all necessary stuff for such a refactoring, may I ask you to replace following things in ./src folder?

  • isInteger (Number.isInteger) --> angular.isNumber
  • Array.isArray --> angular.isArray
  • Object.assign --> angular.extend

I believe it should be enough to cover IE 9+ and then I will accept PR.

@evanlok
Copy link
Contributor Author

evanlok commented Jun 29, 2018

@dhilt I've refactored the functions you specified above. I'm assuming there won't be any side effects from using isNumber instead of isInteger?

@dhilt
Copy link
Member

dhilt commented Jun 29, 2018

@evanlok A good question. Let me think a bit, maybe I'll return your util method. I have to leave for some days, then I'll make a release. Accepting this PR, thanks!

@dhilt dhilt merged commit 1d6eb1c into angular-ui:master Jun 29, 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.

2 participants