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 failed builds when using thread-loader #1207

Conversation

valerio
Copy link
Contributor

@valerio valerio commented Nov 4, 2020

Possible fix for #1206

When using thread-loader, the loader._compiler field is undefined, this causes a runtime error when trying to use it as a key in a WeakMap.

This PR changes the instance cache so that it now uses a dummy "marker" object as reference when the compiler instance is undefined.

Copy link
Member

@johnnyreilly johnnyreilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thank you!

Now I must reveal a prejudice of mine... I don't really like classes. It's not an accident that ts-loader is pretty much a class free codebase.

From the looks of it, class InstanceCache is a class that is instantiated just the once. I wonder if it would be straightforward to switch over to the revealing module pattern here perhaps?

No worries if you don't have time now, this looks functionally completely correct.

@valerio
Copy link
Contributor Author

valerio commented Nov 4, 2020

From the looks of it, class InstanceCache is a class that is instantiated just the once. I wonder if it would be straightforward to switch over to the revealing module pattern here perhaps?

Fair point! Switching to that should be fairly easy, I'll make the change 👌

@johnnyreilly
Copy link
Member

Awesome - you want to include the package.json and CHANGELOG.md changes as well? All good and we'll ship!

@valerio
Copy link
Contributor Author

valerio commented Nov 4, 2020

Awesome - you want to include the package.json and CHANGELOG.md changes as well? All good and we'll ship!

Done!

I'll also take a look at the other error that I saw when using thread-loader and open an issue for it (unless there's something related already).

@valerio
Copy link
Contributor Author

valerio commented Nov 4, 2020

@johnnyreilly I've unlinked this PR from the original issue (#1206), so that it can be closed after users report that it's fixed, but feel free to close it if you prefer handling it that way.

@johnnyreilly
Copy link
Member

Travis is stuck - going to override and merge.

@johnnyreilly johnnyreilly merged commit 3f73e98 into TypeStrong:master Nov 4, 2020
@johnnyreilly
Copy link
Member

v8.0.9 should be out in the next 20 mins or so - thanks for the quick turnaround @valerio!

https://github.com/TypeStrong/ts-loader/releases/tag/v8.0.9

This was referenced Mar 11, 2021
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.

2 participants