LionC / express-basic-auth

Plug & play basic auth middleware for express
328 stars 57 forks source link

Move @types/express to devDeps & type-check the .d.ts file #5

Closed toverux closed 6 years ago

toverux commented 7 years ago

Three little commits, three little things :

LionC commented 6 years ago

Move @types/express from deps to devDeps, as discussed in #4. Works fine if the client has @types/express in it's own dependencies, which should be the case. Otherwise, that user isn't working in strict mode and having or not @types/express won't make any difference.

Can you clarify that? I think I do not fully understand the consequences you are describing.

toverux commented 6 years ago

I think that there are two main cases:

Worst case scenario: TypeScript fails, giving a clear and unambiguous message saying that @types/express should be installed.

LionC commented 6 years ago

Merged, thank you

vjpr commented 5 years ago

@toverux

He/she has likely installed express before express-basic-auth, and @types/express alongside because otherwise he/she couldn't have used express in the first place

This is a bad assumption. To be valid, a pkg must explicitly define its dependencies. Otherwise you are relying on hoisting. This will break pnpm and Yarn PnP.

toverux commented 5 years ago

Yes, but TypeScript was a special case. Lots of people were doing that, and maybe still do. Indeed, we were relying on hoisting since it was a known behaviour of all the package managers at the time. Maybe yarn pnp is a bit young if it's still so fragile and intolerant? :) According to you, we should put @types/express in dependencies? How are other packages doing today?