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

Adjust package for Create React App use case #1

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

Timer
Copy link
Owner

@Timer Timer commented Oct 21, 2018

No description provided.

superlbr and others added 24 commits October 21, 2018 20:11
* Add compilerOptions option

* Add unit tests

* Update version and changelog

* Update README.md

Co-Authored-By: ianschmitz <ianschmitz@gmail.com>
…Strong#175)

* chore(deps): update lodash, mocha away from vulnerable versions, fix associated code and npm scripts, update tslint config

* chore(config): use full es2015 instead of 2015.core+es2016 in tsconfig libs
Merge in compilerOptions prior to calling parseJsonConfigFileContent
@johnnyreilly
Copy link

johnnyreilly commented Oct 30, 2018

Hey @Timer,

I was just taking create-react-app 2 for a spin using the TypeScript support. When I ejected I discovered that it wasn't using the fork-ts-checker-webpack-plugin but using this fork instead.

It may be that your changes are very CRA specific. If there's changes here that are worth making in fork-ts-checker-webpack-plugin directly, we'd love to know! A fork is often a sign of a deficiency in a package; and we'd be keen to solve for that!

Also, I'm imagining maintaining a fork isn't what you want to spend your time doing 😄

@Timer
Copy link
Owner Author

Timer commented Oct 30, 2018

@johnnyreilly hey! I mentioned this in TypeStrong#165 (comment), but dropping the peer dependency is a hard requirement for us.

If you're OK with dropping the peer dependency, I'm more than happy to refine this PR and send it upstream!

(yes I know this breaks Yarn PnP but I'm OK with this)

@johnnyreilly
Copy link

Ha - sorry I didn't see that comment @Timer!

If you're OK with dropping the peer dependency, I'm more than happy to refine this PR and send it upstream!

Sounds good to me!

@piotr-oles (who originally created fork-ts-checker-webpack-plugin ) is happy provided the following requirement is met:

If we want to remove it from peerDeps, we should add some checks (for typescript package and it's version) in the plugin code to inform user that typescript is not available and he has to install it to use this plugin. :)

Does this approach work for you? If so I'd be more than happy to look at a PR.

@Timer
Copy link
Owner Author

Timer commented Oct 30, 2018

Yes, I'm happy with that approach and checking the version!

Will PR soon!

@johnnyreilly
Copy link

Thanks!

@Andarist
Copy link

@Timer could u summarize shortly what needs to be done? I could probably work on this

@Timer
Copy link
Owner Author

Timer commented Jan 11, 2019

I believe @johnnyreilly has a PR ready-to-go in the CRA repo, pending one final review.

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.

6 participants