Skip to content
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

FYI: existing type definitions #107

Closed
JeffJacobson opened this issue Jan 29, 2018 · 6 comments
Closed

FYI: existing type definitions #107

JeffJacobson opened this issue Jan 29, 2018 · 6 comments

Comments

@JeffJacobson
Copy link
Contributor

A while ago I contributed the @types/arcgis-rest-api type definitions to the DefinitelyTyped repository. If they meet your needs you could use these instead of creating them from scratch. (Available via NPM here)

I have another version with additional types, but that hasn't been contributed yet. I've attached that version below, in case you want to use it.
arcgis-rest-api.d.ts.zip

(The only reason that all of the types are in a single index.d.ts file is simply because that's the way the DefinitelyTyped project expects them to be.)

@jgravois
Copy link
Contributor

jgravois commented Jan 29, 2018

very cool! this is really similar to the arcgis-rest-common-types package we recently started stubbing out.

we could certainly link out and promote your additional typings, but my first instinct is that it'd be good to combine forces. would you be interested in that?

@JeffJacobson
Copy link
Contributor Author

Yeah, that sounds good. Seems like it would be best to just copy the types I made into your project rather than linking out to the @types ones, so they would be kept all in a single location.

@tomwayson
Copy link
Member

tomwayson commented Feb 21, 2018

@JeffJacobson this is awesome - I'm starting to recreate some of these - and I don't want to.

No matter what we decide, I think the main thing is to eliminate any duplication of error.

I'll be the first to admit that I'm mystified by TS best practices, I thought DefinitelyTyped was out, and npm i @types/whateves was in. Now it appears you're telling me they are the same thing.

-->Mind blown<--

Also, we've prefixing all interfaces w/ I, but you haven't done that. What gives bro? I like yours better. Is that cool? Can we do that?

My only concerns w/ using DefinitelyTyped are how difficult is it to keep the types there up to date and how do update the types as you develop? Who merges PRs to DefinitelyTyped? I presume each of the libraries in this monorepo would add @types/arcgis-rest-api to their package.json files, but what do we do while we're developing - npm link to a local copy of DefinitelyTyped (again, you have to treat me like a child when it comes to TS). If those concerns aren't a big deal, I say that this library should depend on @types/arcgis-rest-api and start updating them as we need. That way whatever libraries already depend on those can benefit from the updates.

@tomwayson
Copy link
Member

OK, wish I had done a quick google search on this before my previous comment. Turns out, If your package is written in TypeScript then you should publish the types w/ your package, so I'm going to PR @JeffJacobson's types to arcgis-rest-common-types - at the very least the ones I'll need for the new package for managing features. Also I talked IRL w/ @jgravois and he prefer's the I prefix for interfaces, so I'll add that to the ones I bring over.

@JeffJacobson if we were to include all of your types (not just the ones we need), would that be something you'd be willing to use in your projects instead of @types/arcgis-rest-api, even if their prefixed w/ I? I think it'd be great if we could just maintain one package going forward. Hopefully it would be easier for you to publish here than over at DefinitelyTyped. Ultimately I'd like to deprecate the former and update it's dependents.

@JeffJacobson
Copy link
Contributor Author

Sounds good.

@tomwayson
Copy link
Member

closing as this was resolved in #121 - thanks again @JeffJacobson!

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

No branches or pull requests

3 participants