serviejs / popsicle

Simple HTTP requests for node and the browser
MIT License
246 stars 19 forks source link

Use @types instead of typings #106

Closed cprecioso closed 6 years ago

cprecioso commented 7 years ago

Closes #97

This PR updates the project to use typings taken from the @types packages in the npm registry. Everything is fairly straight-forward, except the methods package used for testing, that has no @types package, so I created a custom typing.

It also exports the typings directly through package.json, now that it does not rely on the typings tool to build them.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling 9aee49c95aaed1027838fedab8d1f97687c5897a on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

blakeembrey commented 7 years ago

There's no rush to merge this as a breaking change - do you think we can get methods into DefinitelyTyped to replace the custom types also?

blakeembrey commented 7 years ago

And thanks, by the way! It looks great 👍

cprecioso commented 7 years ago

I made a PR for the methods typings in DT: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/17422

cprecioso commented 7 years ago

do you think we can get methods into DefinitelyTyped to replace the custom types also?

Done!

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling cfc0e1da281ee2f2f50314a618b8a6b775574e02 on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling 2f16a2b1cb980fcc7b98bbe96980299ef7cc7e84 on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling 4413c2cb454f54ef5e8205a200548de0edd434a8 on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling 4413c2cb454f54ef5e8205a200548de0edd434a8 on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.822% when pulling 4413c2cb454f54ef5e8205a200548de0edd434a8 on cprecioso:@types into 7c9f89f413aa8562e3866ed2e222c5e5995a08b8 on blakeembrey:master.

blakeembrey commented 7 years ago

I wanted to merge this, but I also just realised that it does entirely break the browser use-case and it's impossible to resolve until TypeScript supports it. Why? Because form-data is exported from the main package and it relies on the node.js typings to work. If we could somehow make sure form-data is not exported (instead creating a generic interface that works for both), we might be able to workaround the issue.

blakeembrey commented 7 years ago

/cc @unional with https://github.com/blakeembrey/popsicle/issues/97.

unional commented 7 years ago

Maybe internalize types/npm-form-data as custom-typings?

blakeembrey commented 7 years ago

@unional The main issue is that we'd need to expose those types somehow, whether it's global or not. This works with Typings because I enabled browser resolution separate to main resolution (specifically for this use-case). Currently, I think the only work-around will be trying very hard to make sure only custom interfaces that implement both the browser and server side but expose neither specific features are exported. It's a fragile thing, but it might be the only way for now.

unional commented 7 years ago

Sorry, I misread types/npm-form-data I thought the browser.d.ts was to export it globally.

There was a time I tried to do some browser-spec stuff, but I hit a wall related to webpack.

It probably too much work to get a HCF of both interface... :)

Any news on the browser-spec support?

blakeembrey commented 7 years ago

None. I wouldn't expect it to happen. I think I might take a stab at this interface some time next week (busy this week) unless someone else does it. That will be v10. Then v11 will be a much heavier refactor bring it up to par with https://github.com/serviejs/servie and exposing a lighter-weight browser polyfill when you don't need middleware (e.g. https://github.com/mulesoft/js-client-oauth2/tree/master/src/request).

wtrocki commented 6 years ago

Any chance to get that merged?

blakeembrey commented 6 years ago

I'm going to merge and release it as is, for anyone wanting this they can bump directly to 10.x. I'm going to release 11.x shortly after.