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

[typescript] Move @types/jss from devDependencies to dependencies #9817

Merged
merged 1 commit into from Jan 10, 2018

Conversation

pelotom
Copy link
Member

@pelotom pelotom commented Jan 10, 2018

#9678 added a devDependency on @types/jss, however it exposes types obtained from JSS to consumers of Material UI, so it should really be a dependency. Currently one has to add a dependency on @types/jss to one's own project or the Material UI dependency doesn't type check.

@pelotom pelotom added external dependency Blocked by external dependency, we can’t do anything about it typescript labels Jan 10, 2018
@oliviertassinari
Copy link
Member

This @types dependency question has been raised by @rosskevin in the past. My concern was about bloating the node_modules for people no relying on TypeScript. To benchmark with how the other libraries are handling the issue.

@pelotom
Copy link
Member Author

pelotom commented Jan 10, 2018

That's an understandable concern, but @types/jss is pretty lightweight (as are most typings-only packages).

> du -h node_modules/\@types/jss
 16K	node_modules/@types/jss

@rosskevin
Copy link
Member

After spending more time on this particular subject, I feel like all @types for runtime code should be in dependencies. Typescript projects require them just like the dependency itself, so this just makes trial and error work for TS users.

As a counter argument to the bloat argument:

  • it's a single top level folder
  • @types have very few files
  • these are never included in the bundle

So I see no reason to avoid type in dependencies.

@pelotom pelotom merged commit fa14719 into mui:v1-beta Jan 10, 2018
@pelotom pelotom deleted the jss-types branch January 10, 2018 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Blocked by external dependency, we can’t do anything about it typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants