-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
Use native Object.assign() #418
Use native Object.assign() #418
Conversation
Hi @arusakov, Thanks for the contribution - it's appreciated. That said, I don't see the advantage of taking this PR at the moment I'm afraid. Whilst node 0.12 is being end-of-lifed I'm aware there's people out there who will take a little longer to get to the point of upgrade and so I'll probably wait before making this move for the sake of them. I hope you understand. For what it's worth, |
HI @johnnyreilly, |
That's fine - but you don't speak for everyone 😄 |
Given webpack 2 is dropping support for node 0.12.0 we'll probably look at merging this soon. One question: why "lib": "es2015"? For Object.assign itself? |
@johnnyreilly |
Cool - thanks. Will bump ts-loader to 2.0 as part of the merge. (Probably in a couple of weeks time) |
"noImplicitAny": true, | ||
"noImplicitReturns": true, | ||
"noImplicitThis": false, | ||
"noUnusedLocals": true, | ||
"noUnusedParameters": true, | ||
"suppressImplicitAnyIndexErrors": true, | ||
"strictNullChecks": false, | ||
"lib": [ | ||
"dom", "es2015" // dom for console.log only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably can use es2015.core
.
Also, we can shun away from "dom" like here https://github.com/Microsoft/TypeScript/pull/13453/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so do you think this could be replaced with:
"lib": [ "es2015.core", "scripthost" ]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd certainly rather use that than dom
if that's possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably ["es5", "es2015.core"]
with this declaration
declare var console: {log(...args: any[]): void}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll probably merge and then do some tweaking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnyreilly I will rebase and fix it today
24b6b57
to
7a9034d
Compare
object-assign
dependencepackage.json