Include @types dependencies to "dependencies"

[gagle]

Why this change? 90fd3d4

fs-promise is a third party lib that must include all its dependencies in "dependencies", otherwise typescript consumers will never download the @types.

Right now (2.0.2) is broken because you depend on @types/fs-extra and @types/mz. When I compile I get an error. It doesn't find the @types/fs-extra and @types/mz definitions.

[RyanZim]

@dcharbonnier @SBD580 Can one of you explain this? I'm typescript-illiterate.

[dcharbonnier]

hi, I will investigate tomorrow on how to do that correctly. I noticed the "problem".
in the meantime @gagle do a npm install @types/fs-extra @types/mz --save-dev on your project.
Do not close the issues @RyanZim, you can assign me

[SBD580]

@gagle what are you trying to compile? You shouldn't compile this library if its a dependency of yours.
If your trying to compile this library in order to work on it, then of course install the devDependencies as well.

[dcharbonnier]

no @SBD580 because we re-export the definitions so even to use it you need them https://github.com/kevinbeaty/fs-promise/blob/master/index.d.ts#L8

[gagle]

@dcharbonnier for now I'll stick with version 2.0.0, it works fine

@SBD580 if you use typescript and import this library with the latest version it won't compile because in the definiton of types the modules @types/fs-extra and mz are imported (https://github.com/kevinbeaty/fs-promise/blob/master/index.d.ts#L8-L9) and because they are listed as devdependencies they are not downloaded when you do npm install.

[SBD580]

Your right I'm sorry - missed that one.
possible solution will be to publish separate @types/fs-promise package but its just fill wrong.

[dcharbonnier]

https://github.com/dcharbonnier/fs-promise/tree/feature/bundle-ts @SBD580 ok for you ?

[dcharbonnier]

no, it does not work, it's more complicated than that, will find something else

[SBD580]

If you really think about it, all Types should be devDependencies as they are being used in compile time rather then runtime. So maybe @types/fs-promise package is not such a bad idea after all.

If it was possible to get the dependencies in a nested way (node_modules/fs-promise/node_modules) that would have solve the problem as well, as tsc wouldn't look for types there.

[SBD580]

Look like its a common problem, this issue here is a feature request which would have solved this as well:

microsoft/TypeScript#11917

in the meantime I think an additional package is needed (like the most of the libraries do).

[gagle]

Why not just add the dependencies again under 'dependencies'?

@SBD580 If you really think about it, all Types should be devDependencies as they are being used in compile time rather then runtime. So maybe @types/fs-promise package is not such a bad idea after all.

You need to provide the type definition files somehow, otherwise don't give support to TypeScript and let other specific modules provide this. In fact there's a @types/fs-promise module that does exactly that but because you are including a definition file inside fs-promise, it takes precedence over the other one so even if you use @types/fs-promise and fs-promise, fs-promise won't work.

Why do you care so much in what is included in the package? If you give TS support you need to include the definition file as dependency even if it's only a development dependency, this is the trade-off of TS dependencies.

[dcharbonnier]

@gagle because maintaining a version of the definition need to be sync with a project, it's not an aside project. For the current verison you need to add npm install @types/fs-extra @types/mz --save-dev on your own project an it works, it's what I did. For the version that will not require it, typescript is working on solutions enbeded directly in the compiler and I'm also working on it, next release will make it easier.

[kevinbeaty]

All, thanks for the discussion and work on this. To answer the original question of why this was removed, see #22 and related issues.

I am also "typescript illiterate" so can't offer much insight, but happy to include a solution that isn't intrusive for use without Typescript and doesn't cause issues for dependent libraries with Typescript. I would, however, be hesitant to include a solution that requires a lot of effort to maintain the types.

It seems like installing dependent type definitions isn't that much effort for people who want typescript support. It is annoying, for sure, but I think better than the previous behavior of difficult to track down errors due to dependency issues Opt-in seems better than opt-out in this case. What would be the disadvantage of leaving as is and adding a section to the README about Typescript support?

[gagle]

I don't see the problem from including the ts definitions in the library apart from downloading some extra files. Lots of libraries don't ignore correctly the content that they later publish to npm, for instance the test directory should not be included when you're implementing a module that will be imported in another module because you'll never want to test somebody else's modules, you're asuming that they are correct and here I see a bigger problem in the Nodejs community than just including a couple of type definition files that are expected to be downloaded when you install dependencies.

This is a problem that have all the TS modules that depend on @types modules. This problem is about expectations. If I'm using fs-promise and I import the module from TS (because fs-promise supports TS), I expect to get all the dependencies without caring if it depends on another @types modules. For me is not a good solution to put a warning in the readme. For me this is a "hard" dependency, it's not a devdependency nor a peerdependency. In order to make fs-promise to work with TS you need to include them as dependencies because fs-promise depends on them. Conceptually it makes sense.

[kevinbeaty]

@gagle Please see related issues for problems that including the definitions in deps caused for this issue specifically. I am more inclined at this point to remove Typescript support altogether than to include something that will cause compilation problems.

[RyanZim]

I am more inclined at this point to remove Typescript support altogether than to include something that will cause compilation problems.

A hearty 👍 to this.

fs-extra doesn't include them (https://github.com/jprichardson/node-fs-extra#typescript); can't see why fs-promise can't do the same.

[gagle]

@kevinbeaty I see, then for me it's also preferrable to drop support to TS and let people use the "unofficial" @types/fs-promise module.

[SBD580]

I agree. At least until a better solution will be possible with future Typescript releases.

[RyanZim]

OK, it seems consensus is to remove typescript support.

However, this presents a problem with versioning. Currently, fs-promise and fs-extra's major versions are in sync. Removing typescript support would be a breaking change requiring a major version bump. @kevinbeaty Thoughts on how to handle this?

[kevinbeaty]

@RyanZim yeah, I was wondering about that too. I'm thinking about bending the rules a bit here since it is an early 2.0.x release and TypeScript never worked without issues anyway. I think the majority of current users are likely not depending on it and we never did document that we supported it.

[kevinbeaty]

I'm not too concerned about versions being out of sync. They are only numbers. But it is kind of annoying to upgrade major versions when nothing really changed....

[RyanZim]

Forgive my lack of typescript knowledge, but would it work to leave things as they are until the next major release, when the typings could be removed?

[RyanZim]

Also, I want to add promise support to fs-extra sometime soon, so this library is going to be pointless then.

[dcharbonnier]

https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

If you control the npm package you are publishing declarations for [...] bundling with your npm package [...] is favored

and this make a lot of sense.
Reading all the recommendations the first option of the version 2.0.0 is the recommended one and we should never move to the dev dependencies, my fault.

[...], we used "dependencies" and not "devDependencies"

[SBD580]

@dcharbonnier of course this is the desire solution. If the library depends on those Types those should be included inthe "dependencies" section rather then on the "devDependencies".

But the fact is that this will cause other Types to be downloaded as well to the same node_modules/@types directory such as @types/node and this is causing trouble in environments where node is not really available (browser, nativescript etc). In some cases users may use symbols which are not really available during runtime and in other cases a collision between types will be made.

[dcharbonnier]

are you using mz/fs and fs-extra without node or on the browser ?
This is just the best solution even for types "collision", the @types/fs-promise package is not up to date and will never be, if you use it you will get in trouble at some point. the best way (not perfect) is to use the fs-promise types.

[kevinbeaty]

I really don't see a good option here. Seems like we're damned if we do, damned if we don't due to microsoft/TypeScript#11917 . We did see real issues with including the types in deps, so it's not like this is a theoretical problem.

I think I might merge #27 , publish as a patch release since the move to devDependencies seems now like a breaking change and should have never been a patch release in the first place. I then will remove Typescript support altogether and publish a new release, likely 3.0, unfortunately. The only reason I agreed to adding Typescript support in the first place is I thought it would be unobtrusive and require little maintenance. That has not turned out to be the case.

If Promise support is added to fs-extra, I am probably going to create a final release that simply exports fs-extra and deprecate fs-promise in favor of it.

fs-promise (and any-promise) are showing their age, released at a time before Promises were seen as the future, and now Promises are nearly universal. And this is a good thing.

[RyanZim]

@kevinbeaty Sounds like the best plan so far. Just add a table near the top of the README that points out what fs-promise versions use which fs-extra versions.

[gforceg]

Given that this is a thin wrapper, couldn't you just write it in typescript, compile it, and publish it?
Then it would include the typings through the compilation process.
Maybe it's more complicated than that...

[RyanZim]

This issue is pretty well pointless since fs-extra is going to have promise support in the next major release: jprichardson/node-fs-extra#403. No point even doing anything here; this library will be unneeded in just a few days. We're hoping to get this shipped sometime in the beginning of next week.

[gagle]

Closing this, use fs-extra w/ @types/fs-extra