Moving from dt/node to env/node

[nickzelei]

Hey,

I'm posting this here as I didn't know where else to post..

I just moved from the dt/node@4 to the env/node@4 and the latest change to env-node has broken my builds. I'm using the dt version of express with "express-serve-static-core" virtual module.

That module is not playing nicely with the recent change to node-env (as it shouldn't) since it is referencing deprecated http modules. What is the recommended upgrade path to get express working again?

Referencing this pull: #748 and this issue: env-node #34

[felixfbecker]

@Zelein the express-server-static-core typings are simply wrong. They use the ServerRequest typing for req, when it really should be IncomingMessage. https://github.com/types/npm-express should behave correctly, I advise you to use these typings instead of the DT express typings.

[felixfbecker]

Just ran a build with the updated typings, everything is green 👍
https://travis-ci.org/types/npm-express/builds/157127590

[nickzelei]

Okay, great. Thanks @felixfbecker. I will work on switching that over. I'm having problems integrating the new typings express with some custom Swagger types. But I will work that out.

[felixfbecker]

One way they differ is that they do not add body to req by default. You have to manually combine types where you used a middleware that modifies req, like req: Request & ParsedAsText.

[nickzelei]

Is there a way to augment the express Request type?

[felixfbecker]

No, you have to do it explicitly with union types. It allow you to selectively only enable properties on req where you actually used the middleware.

[nickzelei]

I thought this was possible to do with Declaration Merging? I was able to previously extend the Request type in a separate .d.ts file to include the properties that were being patched onto the Request object in the middleware stack.

Forgive me if this has changed, I am still learning the ins and outs of TypeScript.

[felixfbecker]

I'm sure it would be possible with some workaround, but honestly it's a bad pattern. The properties are only available when you actually used the middleware, so you should also type Request explicitly with the added attributes

[nickzelei]

so you should also type Request explicitly with the added attributes

And by that you mean anywhere I am using express.Request, I have to extend it with a Union like you stated above?

[felixfbecker]

yes :)
if you have a middleware that is applied to your own app, you could also make it easier by defining a type alias for this or writing an interface that extends from express.Request.

[nickzelei]

Thanks for the help Felix. It really is unfortunate that even for TypeScript 2.0, the @types/express is using what is currently in DefinitelyTyped, not the correct one that is in Typings.

[felixfbecker]

Yes, they publish from DT, even though types has much higher quality typings. See microsoft/types-publisher#51 and microsoft/types-publisher#4

[blakeembrey]

@Zelein Sorry for that deletion. I run into the bug myself when I started using npm~express yesterday and wanted to make sure no one else hits the deprecated pattern. Because, like I did, it's likely people don't check the docs and used the types that made the most sense.

I also started following the intersection pattern over union which is really much nicer and can avoid some issues around when a middleware is unused. I still use module augmentation in one place, which is for a global middleware function.

Should we close this and log an issue on DefinitelyTyped to stop using the deprecated http interfaces?

[felixfbecker]

I also started following the intersection pattern over union which is really much nicer

Could you elaborate? This is only available in TS 2.0, right?

[nickzelei]

Thanks for the response.

Sure, close the issue out. Something definitely needs to be done to sync
the two declaration files. At best, the DT node types should become the
typings types.. And I've read through the issues Felix outlined.

I noticed recently Webstorm telling me http.ClientResponse has been
deprecated by incoming message but I thought it was coming in 6.X (we are
using LTS) so I mentally scheduled it to be done during that upgrade. I
didn't know (and most people probably don't realize) that it is already out
of 4.x.

On Sep 2, 2016 18:19, "Blake Embrey" notifications@github.com wrote:

@Zelein https://github.com/zelein Sorry for that deletion. I run into
the bug myself when I started using npm~express yesterday and wanted to
make sure no one else hits the deprecated pattern. Because, like I did,
it's likely people don't check the docs and used the types that made the
most sense.

I also started following the intersection pattern over union which is
really much nicer and can avoid some issues around when a middleware is
unused. I still use module augmentation in one place, which is for a global
middleware function.

Should we close this and log an issue on DefinitelyTyped to stop using the
deprecated http interfaces?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#749 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACTt0WnrPkIZ4ShYpnIlMU24QKmRkYpXks5qmK8KgaJpZM4JzxRs
.

[blakeembrey]

@felixfbecker Sorry, using express.Request & bodyParser.parsedAsUrlEncoded.

@Zelein The odd thing is, it's been out forever (even 0.10 used IncomingMessage and doesn't have any ClientResponse or ServerRequest classes). I haven't done the detective work, but I'm guessing the implementation in node.d.ts was just wrong and no one wanted to fix it because it breaks everyone - but better sooner than later (and it's already years later!). See https://nodejs.org/docs/latest-v0.10.x/api/http.html.

[nickzelei]

Wow, yeah. It will probably break a lot of people's TS projects. It should probably be done at some point, especially once the TS 2.0 types gets figured out. Ideally the typings version will get the preference since it is the most accurate.