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

fix: add fork-ts-plugin as a dependency #6401

Closed
wants to merge 1 commit into from
Closed

fix: add fork-ts-plugin as a dependency #6401

wants to merge 1 commit into from

Conversation

elevatebart
Copy link

previously referenced but not dependent.
closes #6400

previously referenced but not dependent.
closes #6400
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@ghost
Copy link

ghost commented Feb 11, 2019

I think this is a bad idea. I don't use TS, why should I install this plugin?

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@elevatebart
Copy link
Author

Hey @demkovych, I see your point.
What do you suggest as a solution?
Remove @deftomat optimization?
Create a plugin system?

@ghost
Copy link

ghost commented Feb 11, 2019

@elevatebart I don't know. As i see, we can't use fork-ts-checker-webpack-plugin without typescript installed. Maybe will be better to move this plugin to devDevendencies and move plugin import under https://github.com/deftomat/create-react-app/blob/321d960a67ad3dba8633adfe3292f14c281db7d8/packages/react-dev-utils/WebpackDevServerUtils.js#L141 ?

@elevatebart
Copy link
Author

elevatebart commented Feb 11, 2019

The problem of the dependency issue will still occur if someone is using typescript, without having this plugin as an explicit dependency, react-dev-utils will break again.

This eslint rule would have caught this issue

@ghost
Copy link

ghost commented Feb 11, 2019

Or rollback to "react-dev-utils": "7.0.1"

@elevatebart
Copy link
Author

That was my reflex too. Hope someone checks here soon.

@deftomat
Copy link
Contributor

My apologies guys. That's the "classic" monorepo issue. I forgot to add it into dependencies. 😞

@deftomat
Copy link
Contributor

Looks like #6395 is a better approach.

@ianschmitz
Copy link
Contributor

Hey just looking into this now.

@elevatebart
Copy link
Author

It still adds the ts-checker dependency to people without typescript... Oh well...

@ianschmitz
Copy link
Contributor

We decided to revert #5903 to fix react-dev-utils for downstream consumers. This will give us a bit of time to figure out the best solution for how to consume the fork-ts-checker-webpack-plugin dependency.

2.1.5 is now available

@ianschmitz ianschmitz closed this Feb 11, 2019
@johnnyreilly
Copy link
Contributor

Heya,

I can see that #6395 has been merged but it also says here that #5903 was reverted.

Could you clarify where are things now please? I'm keen to get the improved TypeScript experience out there in a way that doesn't impede people who don't want to use TypeScript...

@lock lock bot locked and limited conversation to collaborators Feb 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dependency in react-dev-utils
5 participants