-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
#621 kills ts-loader dependant builds #626
Comments
First of all: don't worry about it. These things happen; I've done it before and I'll probably do it again. What matters now is dealing with the issue so people aren't too impacted. Would you be able to submit a PR that reverts #621? I'm on a train right now and not near a computer. If you could do that then I should be able to get 2.3.6 out in pretty short order. |
Will do. FWIW, I know that those things happen, they happen to me as regularly as they probably happen to any programmer. However till this date I've only broken internal tools or, worst case, client websites. Never tools used by thousands of people My head's fine with things like that happening but my emotions can't currently deal with producing mistakes just as stupid as that. 😉 |
Changes reverted in #627. Only reverted my commits of course, other changes of the release haven't been touched. Btw, my changes apparently had sneaked in a |
Another btw: Reverting pull requests seems to be possible with built-in GitHub features. 👍 |
Nice! Is that what you used to create the PR? |
Nope, I googled that after submitting the pull request. I really just did two simple |
Fixed with v2.3.6 |
Hey @loilo, Just a heads up, I'm planning to release ts-loader v3.0.0 in a month or so. It's going to be a major version change as I'm planning to drop support for TypeScript 1.x. (No-one is using ts-loader for that AFAIK and we can simplify the codebase if we do that.) As part of that I'll plan to drop support for |
Hey @loilo , with version 2.3.5 we have had issues compiling our sources with webpack + ts-loader, since modules could not be resolved: I have seen that you have reverted that in 2.3.6. Today I checked again with the latest version (2.3.7) but the error still exists... Our project structure is basically the following:
In webpack.config we configured ts-loader with and in tsconfig.json we only have Could you provide some information about what the correct tsconfig-configuration is? Thank you! |
Hey @psurrey, as per configuration description, the correct configuration in your case would be
You may even just provide the file name However, I'm not sure how your configuration could ever have worked, even in 2.3.5... Let me know if this worked for you. :) |
Thanks for responding @loilo 👍 |
The motivational maintainer assistance is real. 😁 |
Thank you guys for responding. I tried to reproduce my issue in a new project, but wasn't able to do so. So now I'm still trying to figure out why it doesn't work in my actual project 🤔 @loilo The issue occurred with version 2.3.5. So version 2.3.4 works for me, but versions >2.3.5 don't... |
Hum. I guess then some repro repo would be useful to pin this down. |
Okay, seems that I've screwed up with #621.
TL;DR: We probably should revert #621 and release 2.3.6 as soon as possible, subsequently rethink the solution for #619.
What happened?
My assumptions about the
basePath
overriding therootDir
were wrong. If I'd read the TypeScript documentation more carefully, I should have noticed thatrootDir
's only use is "to control the output directory structure with--outDir
". That means, it's not taken into effect if nooutDir
option is specified.Not taking this into account, my added comparison test does not use
outDir
and therefore did not cover this possible point of failure:The
rootDir
gets appended to thebasePath
if anoutDir
is specified. Therefore, setting thebasePath
to the value of therootDir
will inevitably cause the build to break.What to do?
It's probably not too hard to think of an alternative solution, but to have
ts-loader
dependants breaking for as few people as possible, we probably should revert the changes from 2.3.5 and publish 2.3.6 as soon as possible. I'm aware that this now is a real breaking change but I can't think of a better solution from the top of my hat.The current implementation will have to go away anyway since it's fatally flawed, I guess this is pretty much worst case for anything using semver.
Again, I'm really sorry about this.
The text was updated successfully, but these errors were encountered: