-
Notifications
You must be signed in to change notification settings - Fork 30k
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
[lodash] change weakmap declaration to work with TS 2.2 #14502
Conversation
lodash - change WeakMap declaration - fix DefinitelyTyped#14324
Revert "lodash - change WeakMap declaration - fix DefinitelyTyped#14324"
lodash/index.d.ts to authors (@bczengel @chrootsu @stepancar). Could you review this PR? Checklist
|
Typescript 2.2.0 is in RC and npm is alrady downloading it based on the previous dependency version rules. VSCode is currently using 2.1.5, so it seems to make sense to follow that version, additinally the 2.2 compiler has a breaking change with lodash that prevents building spellchecker if tsc 2.2 is used. cf. DefinitelyTyped/DefinitelyTyped#14502
Please, verify it %) |
What do you think about I'm just thinking about how custom |
|
I can't use lodash without this fix... |
Any ETA on getting this merged? |
This pull request is dirty since contains another junk commits. I've made another one in pull request #14662. I also follow @eschwartz suggestion to use |
Pinning to TS 2.1 due to: DefinitelyTyped#14502 Versions are otherwise aligned with Griddle 0.7.1: https://github.com/GriddleGriddle/Griddle/blob/d8f7ee8a8474530b456d9a973a5db743cfb2a9ff/package.json
In TS 2.2
WeakMap
interface for ES6 has been changed fromWeakMap<K, V>
toWeakMap<K extends object, V>
. This changes makes@types/lodash
not working with new ES6 introduced in TS2.2.To keep the backward compatibility with ES5, I think it is safe to rename the
WeakMap
declaration.I know TS 2.2 is still in RC version now, but many people using it, including me :). So if you think this is too early and also bad idea then feel free to close this PR without merging it.
If this get merged then it's should close #14324.