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

Change how TypeScript dependency is handled #165

Closed
brunolemos opened this issue Oct 9, 2018 · 5 comments
Closed

Change how TypeScript dependency is handled #165

brunolemos opened this issue Oct 9, 2018 · 5 comments

Comments

@brunolemos
Copy link

brunolemos commented Oct 9, 2018

We are trying to add TypeScript support to create-react-app, but there is currently one pending blocker: The maintainers don't want to add the typescript dependency to their repository directly, they want the users to add them on a project basis, allowing people to specify the version they want.

The problem is when I remove the typescript dependency from create-react-app react-scripts package (or even try to add it to peerDependencies), fork-ts-checker-webpack-plugin fails with the following error:

Error: Cannot find module 'typescript'
    at Function.Module._resolveFilename (module.js:547:15)
    at Function.Module._load (module.js:474:25)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/brunolemos/Projects/packages/create-react-app/node_modules/fork-ts-checker-webpack-plugin/lib/CancellationToken.js:6:10)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)

Any help is appreciated so we can merge facebook/create-react-app#4837.

I see there is #109 but I don't believe that will be enough to fix the issue.
The error points to this line: https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/237b1fd96fe5cd9450c8189a5c6f62bc5d119ba0/src/CancellationToken.ts#L5

@Timer
Copy link

Timer commented Oct 9, 2018

Just to add, the tslint dependency would be great to go as well. We don't want unnecessary peer dependency warnings.

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 9, 2018

If we can help, we will! But since fork-ts-checker-webpack-plugin needs TypeScript to work I'm not sure what to suggest... Do you have an idea? PRs are gratefully received.

With regards peerDependencies, I suspect they are here to stay because of Yarn Plug'n'Play: TypeStrong/ts-loader#841

@brunolemos
Copy link
Author

We have found a solution on create-react-app's end, so no need to make changes here.
Thanks @johnnyreilly.

@johnnyreilly
Copy link
Member

Really pleased to hear it! Well done @brunolemos !

@Timer
Copy link

Timer commented Oct 22, 2018

Hi, we actually decided against the work around above (mentioned by @brunolemos) and ended up needing to make changes to fork-ts-checker-webpack-plugin.

We're using a fork in Create React App for now and in the next few days I'll probably open a pull request with a proposal to change this behavior.

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

No branches or pull requests

3 participants