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

Set "compilationDone" before "forkTsCheckerServiceBeforeStart" #146

Merged
merged 3 commits into from
Aug 15, 2018
Merged

Set "compilationDone" before "forkTsCheckerServiceBeforeStart" #146

merged 3 commits into from
Aug 15, 2018

Conversation

walkerburgin
Copy link
Contributor

I'm trying to use the forkTsCheckerServiceBeforeStart hook to defer type checking until after compilation has finished in order to pick up typings generated on the fly by a loader. In this case, compilation will be finished before forkTsCheckerServiceBeforeStart is resolved. Right now this.compilationDone will be incorrectly set to false, causing the build to hang indefinitely.

@johnnyreilly
Copy link
Member

@piotr-oles what do you think about this?

@walkerburgin
Copy link
Contributor Author

Anything I can do to help? I'm blocked at the moment on finding some way of accomplishing this (definitely open to better solutions)

@johnnyreilly
Copy link
Member

It's fine with me; but since it represents a change to behaviour I'd like @piotr-oles to review before we merge. He may be on holiday so please bear with us!

@johnnyreilly
Copy link
Member

Ps could you merge in upstream changes to your branch?

@piotr-oles
Copy link
Collaborator

Looks good :) Thanks

@piotr-oles piotr-oles merged commit aea60e1 into TypeStrong:master Aug 15, 2018
@walkerburgin
Copy link
Contributor Author

Any chance we can get a release with this change?

@johnnyreilly
Copy link
Member

Do you want to submit a PR that updates the changelog and the package.json? That will make it easy for me to arrange...

See https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/master/DEPLOYING.md for context

@walkerburgin
Copy link
Contributor Author

Sure, opened #152

@johnnyreilly
Copy link
Member

Thanks - release should be out by the time you read this.

Thanks again for your work!

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.

3 participants