Add new loader option "contextAsConfigBasePath", which when set to tr…

[christiantinauer]

…ue, parses to tsconfig file with respect to webpack context path (https://webpack.js.org/configuration/entry-context/#context) instead of the location of the config file. So all relative paths in config file are treated relative to webpack context. This makes configurations possible, where the tsconfig file is located in a npm package or elsewhere in the project structure.

[johnnyreilly]

Thanks for this! Please could you provide a test that exercises this functionality? An execution test is probably a good fit for this. See the tests dir for more info and read the relevant readmes

[christiantinauer]

I have added an execution test now.

[johnnyreilly]

Your changes have broken the validateLoaderOptionNames test as you've introduced a new loader option. Please could you fix this? It should be a simple case of changing the expected 2.6 output here: https://github.com/TypeStrong/ts-loader/tree/master/test/comparison-tests/validateLoaderOptionNames/expectedOutput-2.6 (both files I think)

[christiantinauer]

My bad. Fixed the test.

[johnnyreilly]

This makes configurations possible, where the tsconfig file is located in a npm package or elsewhere in the project structure.

I wonder if you could expand upon this a little. Specifically, I'm interested in a scenario you presently have that this change will unblock / help with. I suspect this is a good idea, but I'd like to understand the benefit of this behaviour better.

[johnnyreilly]

Also, given your editor is got going to privy to the ts-loader change you might find different errors reported by ts-loader in comparison with tsc

[christiantinauer]

This makes configurations possible, where the tsconfig file is located in a npm package or elsewhere in the project structure.

I wonder if you could expand upon this a little. Specifically, I'm interested in a scenario you presently have that this change will unblock / help with. I suspect this is a good idea, but I'd like to understand the benefit of this behaviour better.

I am currently working on a typescript version of create-react-app. The idea behind react-scripts is to have a single dependency in your app-project and get updates/fixes for webpack-configs, jest-configs and so on. You can stay focused on developing your app and not have to worry about the surroundings.

Therefore i want to "hide" the tsconfig-file in the dependencies, so I can update them with new releases or fix them without bothering the end user (except from updating the main dependency). There is already a ts-version of react-scripts. But it initially creates a tsconfig.json file in the created app and loads it from there (so no possibility to update it without interaction from end user). Also the end user is able to change the config file and might break the whole process. The problem with not having the tsconfig.json in the app directory is the "basePath" when parsing the config (this also goes on with tslint).

Long story short, I don't want the config file to be in the app directory but the base path for ts/tslint should still point to the app directory.

Also, given your editor is got going to privy to the ts-loader change you might find different errors reported by ts-loader in comparison with tsc

I solved this problem with a tsconfig.json file in the app directory (I know :)) and using the extend setting. Similiar to the suggestions of create react app for eslint with editors.

{ "extends": "./node_modules/ts-config-react-app/index" }

[johnnyreilly]

Thanks for the explanation - that's super helpful. I'm sick as a dog right now and will review this properly when better. I think this will probably be merged unless something suddenly occurs to me. It seems fine as long as it's behaviour that lives behind an option.

Whilst I pop off to cough up a lung do you want to update the readme with your option? Ta. Probably worth mentioning that the behaviour may differ from tsc with this option in play.

[christiantinauer]

I have added the description of the option to the README. Thanks for having a look.

Get well soon.

[johnnyreilly]

Hey @christiantinauer,

I'm now feeling slightly more human and so I'm starting to look at PRs.

I solved this problem with a tsconfig.json file in the app directory (I know :)) and using the extend setting. Similiar to the suggestions of create react app for eslint with editors.

It's quite possible I'm misunderstanding you, but the purpose of this PR is so you don't need a tsconfig.json but in order to use it you need to add a tsconfig.json

I'm super confused 😀

I am currently working on a typescript version of create-react-app.

Did you know that this already exists?: https://github.com/wmonk/create-react-app-typescript ?

[christiantinauer]

It's quite possible I'm misunderstanding you, but the purpose of this PR is so you don't need a tsconfig.json but in order to use it you need to add a tsconfig.json

I'm super confused 😀

The purpose is so you don't need it. And for the ts-loader that is true. But if you have a look at the things I added to the README, you will see that editors like VS Code will have problems when there is no tsconfig around.

Did you know that this already exists?: https://github.com/wmonk/create-react-app-typescript ?

I know and I linked it in one of the comments above.

There is already a ts-version of react-scripts. But it initially creates a tsconfig.json file in the created app and loads it from there (so no possibility to update it without interaction from end user). Also the end user is able to change the config file and might break the whole process.

Their are some differences between his and my approach:

• I support both (js and ts) for easier transitions. (The company I work for already had a project, so I started to rework "modules" of it).
• I use TS to Babel. (Makes upstream integration easier and I can follow the development. TS is the "typingoverlay".)
• I want to have full control over the tsconfig.json in the future (reason for PR). (He just adds the tsconfig.json to the project initially and the end user has to update it if necessary. Also the end user can potentially break the scripts with changes to the tsconfig.json.)

[johnnyreilly]

The purpose is so you don't need it. And for the ts-loader that is true. But if you have a look at the things I added to the README, you will see that editors like VS Code will have problems when there is no tsconfig around.

I have to say; I do struggle with this a little. I want to try and explain why. It feels "wrong" that with this setup that to actually do any development your first step has to be to create a tsconfig.json that extends another hidden tsconfig.json. Whilst the code will compile without that, no-one would actually be productive that way because their editor would be red and shouty until they did this. Also, if the user changed their settings in their new tsconfig.json, the new settings would apply to the editor but not to ts-loaders compilation. This creates a confusing disconnect which is not helpful.

One of the intentions of ts-loader is to, generally, be as similar in behavior to tsc as possible. We're not averse to adding options that will alter behaviour when set but the reason should be really good.

I appreciate this matters to you. This is a small change and so I'm not too worried about the impact on the codebase. However, I would ask you: do you think this is a good idea? I'm on the fence right now.

[christiantinauer]

I totally understand your worries and that the goal of ts-loader is to follow tsc as much as possible. My personal opinion is that webpack-loaders should only handle the files they are given, which is why I like the onlyCompileBundledFiles option. This option is the reason why I want to switch from awesome-typescript-loader to ts-loader. As said this option creates a quite different behaviour compared to tsc.

Also there is absolutly no need for a tsconfig.json in your project if you don't use code editors which depends on it. As you mentioned, the workflow would work without it. Having your config files "hidden" away is a tool design decision (create-react-app) and with the extending pattern I follow their suggestion. It is a quite different approach, but I like it. With the onlyCompileBundledFiles and contextAsConfigBasePath options ts-loader could behave like for example babel-loader. This configuration follows more the original idea of webpack-loaders. Again this is my opinion.

[johnnyreilly]

Sure - thanks for explaining. Let me mull this over for a bit.

[christiantinauer]

Did you @johnnyreilly find some time to think about it?

[johnnyreilly]

Yeah, here's where I am:

Against:

• I don't think this is a very useful option. I don't see other use cases than your own.

For:

• the code changes are really minimal. This is a low impact change.
• it has a test in the execution pack and I don't see this being something that would likely be reverted by future changes
• it would make you happy

Since the pros outweigh the cons I'm planning to merge this change. When I next get time to do a release I'll merge this and ship.

Thanks for your contribution and for bearing with me. 🐻

[christiantinauer]

That is great news. Thank you for challenging my idea/contribution. Also thinking about your "Against" I came to the conclusion that this might not be only my use case, but a more general approach for fully integrated development tools like react-scripts. I guess we will see in the future.

Thank you very much. Have a nice weekend.

[johnnyreilly]

Shipped with 3.2.0 🌻

[jacobdeichert]

I'll point out that I have the exact same use case as @christiantinauer. I made a cli tool (create-app) for myself and I ran into this exact same issue a few months ago but decided i'd keep the tsconfig file in my project src due to editors like vs code needing them.

With this change, i can now extend the config from inside my create-app module instead, just as @christiantinauer described. This is perfect!

@johnnyreilly thank you for merging this!

[johnnyreilly]

Awesome! Thanks for sharing!

[johnnyreilly]

@jakedeichert it sounds like there are issues with this option and it may be replaced with a new option. See #688 cc @christiantinauer

[jacobdeichert]

@johnnyreilly thanks for the notice! I haven't tried to take advantage of this new option yet. I'll wait until a proper fix is in place.