Removed option "contextAsConfigBasePath" because the context given to… #688
Conversation
… the loader is NOT the webpack.context but the directory of the current file. This makes the option useless. As there is no possibility to get access to the webpack.context in a webpack loader, a new option which serves the same purpose is introduced. Added option "context" (string). Allows to directly set the base path when parsing the tsconfig file. Has to be an absolute path.
|
Today I found out that webpack.context != loader.context. The loader.context always points to the directory of the currently handled file. Within a webpack loader there is no possibility to get the webpack.context path. This makes the option "contextAsConfigBasePath" quite useless. Therefore I created this PR which leads to the same behaviour with a different config option. I am trully sorry. |
|
Ah. Well that's a bit of a pain.
How come the test that was added to the execution pack passed then? |
I know.
Well there is only one simple test and it didn't show the problems with the paths. I am currently working on a better test. I will update the PR when its done. |
|
Thanks - I appreciate this was a mistake and you didn't intend it. If you could be extra diligent that the execution test correctly covers this new behaviour that would be helpful. Releasing this change will be a breaking changes release which I don't like to do very often. However, there's likely to be a breaking changes release when I merge this PR: #685 Hopefully this can come in when that does. |
|
Reworked test "option-context". |
|
@johnnyreilly I have completly reworked the test. Maybe it is a good idea to do a quick release before people start to use the option "contextAsConfigBasePath". At the moment there are no issues, connected to the option, reported. |
Sorry - can you elaborate on this please? I think I may update the README.md to mark the |
Not my day. This is a typo (I corrected the comment). I wanted to write no and not now. |
|
Will merge and fix tests afterwards |
|
Will look to release this with 3.3 |
|
sorry to report -- this PR breaks my build :( my scenario: thanks for all your hard work on this! |
|
Hi @zackschuster, I'm afraid that I'm not planning to reintroduce the old option as it didn't fulfill the requirements that @christiantinauer was aiming for. I'm sorry I broke your build; I was under the impression the old option didn't have any practical use and so removing it wouldn't be an issue. I'm sorry it has been. Given that you can fix this by moving to the new option I'd encourage you to do so. Once again my apologies for the misstep. |
|
no problem! thanks for the fast reply :) |
|
Hi @zackschuster, context in the sense of a webpack loader is the containing directory of the currently processed file (which I didn't know, when i submitted the first PR because of a to simple test case). There is no way that I know of to get the "project context" within a webpack loader. Therefore I introduced the ts-loader option context to set it from the outside. I guess your tsconfig.json does not contain paths like in the include/exclude/files setting. Resolving these with respect to the project context did never work. |
|
i'm not sure how my case worked then; i did have paths in include & exclude that weren't resolved unless contextAsConfigBasePath was set. |
|
Does your project contain subfolders? |
|
it's a yarn workspace, so yes: three nested frontends point at a parent |
… the loader is NOT the webpack.context but the directory of the current file. This makes the option useless.
As there is no possibility to get access to the webpack.context in a webpack loader, a new option which serves the same purpose is introduced.
Added option "context" (string). Allows to directly set the base path when parsing the tsconfig file. Has to be an absolute path.