typings / registry

The registry of type definitions for TypeScript
238 stars 176 forks source link

add parseUri typings #813

Closed effervescentia closed 8 years ago

effervescentia commented 8 years ago

Typings URL: https://github.com/effervescentia/typed-parseUri

felixfbecker commented 8 years ago

You know about native url.parse right? That module looks horrible - no readme, commited node_modules, test runner as dependency...

effervescentia commented 8 years ago

@felixfbecker the typings yeoman generator picked up the wrong npm package. Supposed to target https://www.npmjs.com/package/parseUri. And I know about it, but that doesn't mean I can use it in the browser, whereas I can use this. I'll fix up the target later today

felixfbecker commented 8 years ago

@effervescentia there is a port of the native url package for browserify ;)https://www.npmjs.com/package/url

(of course I have no problem with merging this)

effervescentia commented 8 years ago

@felixfbecker using webpack not browserify

felixfbecker commented 8 years ago

Doesn't matter, you can just add that package as a dependency ;)

effervescentia commented 8 years ago

😓 foiled at every turn... It's smaller? Justifiable?

felixfbecker commented 8 years ago

Well it has way more downloads so I would say it is way more reliable/smaller/tested

effervescentia commented 8 years ago

https://www.npmjs.com/package/url has just under twice as many downloads as https://www.npmjs.com/package/parseUri, which solely encapsulates the functionality of parsing a uri, whereas url also has logic for building urls, so I would argue it is demonstrably smaller. Not to mention that parseUris download statistics are not insignificant by any measure.

unional commented 8 years ago

I think it's merely a personal choice. If the typings looks fine I think we should merge it. @felixfbecker ok with you?

blakeembrey commented 8 years ago

I merged since it all looked good and the suggestions here were the same I'd give anyway, it's a pretty barren library - no README, source code links, docs, etc. If it works, great, but I'd be hesitant to tell anyone else to use it too 😄

effervescentia commented 8 years ago

@blakeembrey thanks for merging. However, it looks like somehow it's pointing at https://www.npmjs.com/package/parseuri instead of https://www.npmjs.com/package/parseUri. Is this because of the file naming (parseuri.json)? Or something else? I also see https://www.npmjs.com/package/parseuri as the source when I run typings search parseuri. The generated typings also appear to use the module name parseuri.

blakeembrey commented 8 years ago

Wow, that's confusing. Yeah, it should have been uppercased in the filename. You can submit a PR to correct that.

effervescentia commented 8 years ago

ah damn -__- okay cool, thanks

unional commented 8 years ago

Yeah. Since you use generator-typings, that comes from the source package.json`

effervescentia commented 8 years ago

@unional can confirm that it was the file naming in typings/registry and nothing else (at least, seeing as everything else was correct)

unional commented 8 years ago

I see. You are correct. The typings.json looks fine. https://github.com/effervescentia/typed-parseUri/blob/master/typings.json

effervescentia commented 8 years ago

@blakeembrey kind of odd behaviour though, would have assumed (along with @unional) that the name field in typings.json would be responsible. However I also understand the requirement of wanting the files in the registry to not just be named willy nilly. Could I suggest that perhaps the name field in typings.json be used, but then the name of the file is verified against that name during the test phase to ensure correct naming in both places and keep from having this happen again

blakeembrey commented 8 years ago

Nope, sorry. The name field in typings.json was a later addition - mostly just for logging in the end. I believe the whole project still works without a name at all. The API itself doesn't actually load anything from your definition to serve it, it just provides pointers from a -> b.

effervescentia commented 8 years ago

ah, fair enough 👍 thanks