-
Notifications
You must be signed in to change notification settings - Fork 216
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
Add TypeScript definitions and test #119
Conversation
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.
lgtm!
I'll merge this and deploy as a feature version bump later today. |
package.json
Outdated
@@ -34,6 +35,7 @@ | |||
"rollup-plugin-commonjs": "8.2.1", | |||
"rollup-plugin-node-resolve": "3.0.0", | |||
"tap": "~7.1.2", | |||
"typescript": ">=2.2.2", |
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.
this is a decently old version from some time in early 2017 from what I can see, is there a reason 3.1.1
was not chosen?
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.
Numerous projects still use TS 2. All of the TS language features in the deepmerge typedefs were present in TS 2, so we may as well make it clear that TS 2 users can successfully use deepmerge with these typedefs. We're just setting a baseline here, saying "if you are using TS 2.2.2 or later then these typedefs should work for you."
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.
Yeah, I'm fine with TS2-compatible typings. Until TypeScript adds recursive types, I don't think there's any reason to push forward.
Published as 2.2.0 |
This PR adds working TypeScript definitions (the DefinitelyTyped ones don't work). JavaScript projects can take two approaches to providing TS definitions:
The second approach is recommended by the TypeScript project. For projects that are willing to take on the maintenance of typedefs the first approach seems preferable to me. For projects that would rather focus strictly on JS and let the TS community handle typedefs, the second is clearly preferable.
@TehShrike indicated interest in maintaining typedefs within the deepmerge project, so I am submitting this PR.
index.d.ts
: add typedefspackage.json
typescript
as dev dependency. Why>=2.2.2
?npm install typescript@2.0.2
results in 2.2.2test:typescript
; also added it to thetest
scripttsc.js
directly sincetypescript/bin/tsc
will not run on Windowstests/typescript.ts
: this file contains non-functional code that simply exercises the typedefs. If it compiles without error then the typedefs are not terribly broken (they could still be too loose, e.g. I don't think it's possible to specify a concrete type for the union of an arbitrary number of objects formerge.all
, but I am not a TS master).