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

Shares solution builder between instances #1176

Closed
wants to merge 3 commits into from

Conversation

sheetalkamat
Copy link
Contributor

@sheetalkamat sheetalkamat commented Aug 31, 2020

I am not sure if this right change or not as i dont understand much about config and need for different instances but this came up as part of investigation where multiple instances of solution builder were created because different instances were used. The project in question takes about 2-3 minutes to just check if the solution is upto date adding to perf and memory implications.

I am not sure if we need this PR or people just fix their config file to use same instance instead. (which is what investigation resulted in)

@johnnyreilly
Copy link
Member

johnnyreilly commented Aug 31, 2020

Thanks for both the PRs @sheetalkamat! 🌻

I've looked at both and each seems like a legitimate approach... To your question:

i dont understand much about config and need for different instances

I've never found a need for using different instances myself either; it predates my involvement with ts-loader and I've never removed it. Every now and then someone has contributed tweaks to it (in fact a recent release #1104 was a tweak to instance functionality) so it's a feature that's used; just not by me 😄

Given that it's a feature that's not likely to go away I'm tempted to go with one of the PRs you're suggesting. What are the pros
and cons of each of the approaches you've suggested? They don't seems wildly dissimilar...

@sheetalkamat
Copy link
Contributor Author

This one just shares solution builder and uses program/language service as if its own.. the other one just does not create new instance so program/LS are same across instances in that other PR.

@johnnyreilly
Copy link
Member

This one just shares solution builder and uses program/language service as if its own..

Okay cool. I don't feel strongly about the different approaches; each sounds good! Without having tested it, I'd guess that #1177 may perform slightly better as it's not creating new instances. I haven't tested it and it may be a marginal difference, but maybe #1177 is the way to go.

Do you fancy incrementing the version in the package.json and adding an entry to the CHANGELOG.md on #1177 ?

@johnnyreilly
Copy link
Member

Closing in favour of #1177 which shipped with https://github.com/TypeStrong/ts-loader/releases/tag/8.0.4

@johnnyreilly johnnyreilly deleted the differentInstances branch March 28, 2021 12:51
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.

None yet

2 participants