Skip to content

Move all @types dependencies to devDependencies #479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

WorldMaker
Copy link

I've been seeing "bleedover" issues with @types dependencies installed by typedoc. (In my particular case, it's with @types/lodash not compiling in my projects for various reasons.) Beyond my specific issues, it seems like a good idea to try to avoid contaminating downstream users of typedoc with extra typings they aren't expecting.

@blakeembrey
Copy link
Member

Please search and read the dozen existing issues and PRs first.

@blakeembrey
Copy link
Member

blakeembrey commented Apr 13, 2017

If you have a specific issue, please report it first or at least mention what the issue is in the PR. As it is I'd prefer to see any issues you have, fixed in DefinitelyTyped or TypeScript or wherever your particular one is coming from. Without mentioning it to those particular parties, how will anyone ever fix your issue permanently?

@WorldMaker
Copy link
Author

I searched all existing issues and PRs and did not find relevant issues. There was no existing issue for this PR, as it seemed a simple metadata fix I assumed submitting a PR would double for submitting an issue. On re-evaluation I realize my mistake in thinking this was a simple fix, however your comments aren't particularly helpful to diagnosing my mistaken assumption. At the time I submitted the PR, I thought I had included enough information in the PR description to note what the issue I was having was (my project doesn't use any of the dependencies of typedocs itself and depends on typedocs only as a devDependency, but fails to build because it's getting types that it doesn't need; this may be more a failure of the npm model than DT or TS where I would like my devDependencies segregated further from my actual dependencies...)

Finally, if you would like issues to be filed before PRs, I suggest adding a CONTRIBUTING.md file noting that.

@WorldMaker WorldMaker deleted the patch-1 branch April 13, 2017 22:00
@aciccarello
Copy link
Collaborator

@WorldMaker You're right that it seems like a simple fix but we've already had several PRs and extended discussions in several issues, especially #384 and #350 about this particular issue which is why @blakeembrey suggested adding more info. If you dig into those threads you'll see the reasoning but it is more complicated than I'd like to try to sum up in a comment every time this comes up 😫

Most of the issues are caused by bad DefinitelyTyped definitions so in #350 (comment) it was suggested that someone pin down the @types to get around the lack of semver in @types. A PR for that would be welcome 😄

@WorldMaker
Copy link
Author

Thanks @aciccarello. Yeah, it's frustrating on all sides when simple things turn out to be much more complicated than at first glance and the searches you thought you needed to try were the wrong searches because you were missing the right perspective on the problem.

@blakeembrey
Copy link
Member

You should be able to find existing issues/PRs using https://github.com/TypeStrong/typedoc/pulls?utf8=%E2%9C%93&q=types%20dependencies. Make sure you searched for closed issues/PRs, not just open ones - I know GitHub defaults to open only (for god knows what reason).

Can you please share what about your build is failing? I understand that it's failing, but it's really not enough context to help you or point the issue in the right direction. When you say it's included in your build, only global type definitions under @types can automatically be included in the build - is that the case? I just need more context, then I can help figure out the right solution. Otherwise I spend an hour guessing and pulling together everything it can possibly be for people, which can get time consuming (as I'm sure you understand).

@blakeembrey
Copy link
Member

I see you did find microsoft/TypeScript#11917, which I suspected might be the issue you ended up hitting. I've been pretty vocal about trying to avoid the mistake of automatically including globals where they don't belong (that issue and microsoft/types-publisher#107 (side note: maybe I should remove the question mark in title to make it less questiony), plus many others). I'm sorry this has hit you, but I'm try my best to make sure we can ask for the correct fixes in TypeScript instead of impacting every module author.

@WorldMaker
Copy link
Author

Yeah, I get it a lot better now, Blake. Between two developers on two machines we had three different problems with the lodash globals. Pinning @types/lodash wouldn't even help that much, because the errors were different because of different versions of Typescript and more importantly different versions of lib.d.ts in different parts of the build chains producing different errors. Right now you have to pick a lodash types version extremely carefully based on the lib environment and that is crazy brittle for everyone involved.

@WorldMaker
Copy link
Author

For now explicitly listing only the types we do need in the tsconfig works and it's better than my co-developers approach of commenting out the lodash globals section every time npm reinstalled it.

@WorldMaker
Copy link
Author

Also found DefinitelyTyped/DefinitelyTyped#15331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants