Skip to content
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

Include npmDevDependencies in wantedLibs #340

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

armanbilge
Copy link
Contributor

This is convenient when you don't want downstreams to inherit the dependency to the type definitions. In fact, I would expect this is the case for nearly all facades.

@oyvindberg
Copy link
Collaborator

Hi there @armanbilge , thanks for this :)

We'll find a way to do this, but I have a very full week, so I'll get back to you on this in the weekend or early next week. What you have done is fine, but there are some complications down the road - I'll do a quick writeup then

@armanbilge
Copy link
Contributor Author

Thanks, no rush at all! Coincidence timing with that other issue, I didn't see it :) very curious to hear about these complications.

@oyvindberg
Copy link
Collaborator

Alright, I have a few minutes now. Sorry about the turnaround time, but life is incredibly busy right now :)

The problem i alluded to above is that of circular dependencies. It's Javascript, so if course it happens a lot. As long as we don't follow devDependencies it's easy to resolve, but if we consider those for all libraries it becomes a problem which requires a strategy to work.

I decided to go ahead and do what you propose here for now (and an analogous change to ScalablyTypedConverterExternalNpmPlugin), as it shouldn't cause (much) problems, since it's just the initial set of libraries. The alternative would be to do it for all libraries here, but that's probably what I did in the past which caused a lot of problems.

@oyvindberg oyvindberg merged commit 103b7b9 into ScalablyTyped:master Aug 20, 2021
oyvindberg added a commit that referenced this pull request Aug 26, 2021
…. (Followup to #340)

- CI build ignores `dev` and includes peer (no change)
- CLI tool optionally includes dev and always includes peer (no change)
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.

2 participants