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

Allow the loading of files through loader configuration #385

Closed
wants to merge 1 commit into from
Closed

Allow the loading of files through loader configuration #385

wants to merge 1 commit into from

Conversation

onigoetz
Copy link
Contributor

Hi,

I had the need to provide a default configuration for TypeScript through the loader so a user doesn't have to create a tsconfig.json at all for basic usage of webpack with ts-loader.

I was able to do it for compilerOptions but files are skipped from this configuration.

This pull request adds this feature and does it through the TypeScript API the same way as for normal files.

I also added two tests that ensure this works as expected.

@johnnyreilly
Copy link
Member

It's an interesting idea.

However, I rather think having a tsconfig.json is a good thing. I'm a little hesitant to see what the benefit of this is? Sure it allows people to get up and running without a tsconfig.json, but if they were doing anything beyond "hello world" they'd probably want a tsconfig.json.

I'm open to being convinced but I'm not yet sold that getting going without a tsconfig.json is a great move. Could you explain what the benefit of this is please?

@jbrantly
Copy link
Member

I agree that if the point is "I can stick all config in webpack config and forego tsconfig" then I don't think this is necessary. You should be using tsconfig instead. However, if changed to "is it possible to add/remove files for the webpack-specific build" I think that's maybe a little more interesting. I think it would be nice to have a concrete scenario for that before adding though.

@onigoetz
Copy link
Contributor Author

Wow, that was fast, sure I can explain.

In the company I work for, we have a tool that is like create-react-app : A set of default configurations to get you started for most development cases, and with all company-wide configuration for Webpack, babel, TypeScript, Eslint, tslint, Postcss, image compression, Stylelint ...

The goal of this tool is to manage everything through a single configuration file that hides all the complexity of the tools used below. but still allows to create tool-specific configurations that will be taken in account (tsconfig.json, webpack.config.js, ...)

For example a configuration like this:

module.exports = {
  jsProvidedLibraries: ["react", "react-dom"],
  js: {
    reactTypeScript: {
      webpack: true,
      react: true,
      source: "js/index.ts"
    },
  }
};

Will use webpack with ts-loader to compile the file with all of its dependencies, including css, treat react and react-dom as external, embed everything else, compress the production build, be able to run the tool in watch mode which will spawn a webpack-dev-server on a free port and handle Hot Module Replacement for your application.
All of this without ever changing webpack's or any other tool's configuration.

We also use typings in our project, and I would love to handle on our tool's side the loading of these definition files. so it's not the final user's responsibility to add this in a tsconfig.json:

{
  "files": [
    "./typings/index.d.ts"
  ]
}

I hope this clarifies a bit the reason of this pull request.

@jbrantly
Copy link
Member

I guess I don't understand why your scenario requires files to be specified in webpack.config.js instead of tsconfig.json. By putting stuff in tsconfig.json instead you would enable TypeScript tooling other than ts-loader to also work.

@johnnyreilly
Copy link
Member

By putting stuff in tsconfig.json instead you would enable TypeScript tooling other than ts-loader to also work.

That's also what I was thinking...

@onigoetz
Copy link
Contributor Author

Sorry if I explained myself incorrectly, webpack.config.js is to provide configuration to webpack directly, like new loaders or else.

But writing things in tsconfig.json would be to add enhance the ts-loader specified configuration.

My goal is to be able to give a full configuration to webpack and ts-loader without adding a single webpack or typescript specific configuration file.

@johnnyreilly
Copy link
Member

My goal is to be able to give a full configuration to webpack and ts-loader without adding a single webpack or typescript specific configuration file.

I kind of think this is a bad idea.... Isn't this feature is only useful for "hello world" scenarios? I don't really see it being used for anything else.

Have I missed something? I might be being a bit dense (totally possible!) but I can't really see an advantage to this at the moment...

@onigoetz
Copy link
Contributor Author

No, it also works for full applications.

I think that when you defined your compilerOptions for your project, and the typings file (typings/index.d.ts) is being loaded fine, you don't have to touch your tsconfig.json at all in your day-to-day work, right ?

The only things you do are:

  • install packages yarn add / npm install
  • install typings typings install
  • write code

None of these requires you to change your tsconfig.json.

That's why I want to hide this complexity from the "final developer". I write the build tool wrapper, he writes his application.

@jbrantly
Copy link
Member

That's why I want to hide this complexity from the "final developer". I write the build tool wrapper, he writes his application.

Presumably this developer might want to use a TypeScript-aware IDE. How do you plan on configuring that IDE with TypeScript options without a tsconfig.json?

I'm starting to see what your goal is. I didn't realize create-react-app hid away configuration files; I assumed it was just a scaffolder. However, that project doesn't seem to deal with IDE configuration, only the build process. I think the TypeScript scenario is a little different due to other tools possibly wanting to interact with that "configuration that isn't there".

@johnnyreilly
Copy link
Member

you don't have to touch your tsconfig.json at all in your day-to-day work, right ?

Actually, purely speaking for myself, no. Both as TypeScript changes and as I reach different points of a project I do find myself tweaking the settings. It's not a daily task but it is pretty regular as I try to move to using the most beneficial setup that I can find at a given time.

@onigoetz
Copy link
Contributor Author

How do you plan on configuring that IDE with TypeScript options without a tsconfig.json?

It depends on the IDE, in my case, we all use IntelliJ/WebStorm which is able to automatically find all definition files. Which allows it to detect problems in my code with it's own engine. I have to admit I didn't consider this part for other IDE's for the moment.

I didn't realize create-react-app hid away configuration files; I assumed it was just a scaffolder.

It's both actually, by default it hides the configuration away and with npm run eject it unwraps them and allows you to modify them (but you lose the ability to just update one package and everything is up to date)

It's not a daily task but it is pretty regular as I try to move to using the most beneficial setup that I can find at a given time.

I totally agree with you, it's exactly my way of working as well, but sadly it's not the case for everybody, many think of these tools as "configure once and forget".

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Nov 18, 2016

A better choice is awaiting 2.1.3 for configuration inheritance.

The goal of this tool is to manage everything through a single configuration file that hides all the complexity of the tools used below

This is fairly impossible unless you can patch out every thing you need. For a simple app it's possible but for a project with enough dependencies that's virtually impossible given the blooming ecosystem of JavaScript.

we all use IntelliJ/WebStorm which is able to automatically find all definition files.

This will quickly fail when you want some specific customization or you want to ignore some definition. For example #381
Also, if you are using something like decorator like @Connect in redux, how can you teach IDE to accept experimentalDecorators? strictNullChecks or noImplicitAny also requires configuring. Without tsconfig.json you have to tell your team to change IDE configuration (or just waiting webpack fails).

so it's not the final user's responsibility to add this in a tsconfig.json

ts-loader will load typing files by default if tsconfig.json exists in the directory, well, if tsconfig.json exists.

After all, given you are building a create-react-app, why bother limit yourself to one single configuration file? I wonder how you pack all the eslintrc, babelrc, stylintrc into one file and hoping IDE can configure all them out. Also, if you are targeting end developers who don't care about configurations, well, by definition, they don't care about how many configuration files in your scaffolding and you can go ahead to put more config.

@onigoetz
Copy link
Contributor Author

This is fairly impossible unless you can patch out every thing you need.

It is possible, I did it :D I would love to show you, but sadly I can't open source it. We create online banking applications with it, not just "hello world" apps.
Just to be clear, I don't bundle the final application's dependencies. I bundle only the compilation/bundling dependencies.

This will quickly fail when you want some specific customization or you want to ignore some definition. For example #381

My case is not like this one, you don't have one installation and all packages are symlinked from there, in my case, you have let's say 20 independent projects, each has the same stack (TS/Babel/Webpack/Postcss) and we don't want to maintain the configuration 20 times. We want to take a "convention over configuration" approach.

Also, if you are using something like decorator like @connect in redux, how can you teach IDE to accept experimentalDecorators? strictNullChecks or noImplicitAny also requires configuring.

Yes, exactly, but I never said you should never have a tsconfig.json, I just don't think you should have to create one when you are starting out. Get started with a basic configuration and if you want to enable strictNullChecks and experimentalDecorators you can do so in your tsconfig.json. But this is already "advanced" usage of TypeScript, not necessarily something you do when starting with TypeScript.

Without tsconfig.json you have to tell your team to change IDE configuration (or just waiting webpack fails).

Some parts are handled by IntelliJ, but you are right, some configurations are not inferred automatically.

ts-loader will load typing files by default if tsconfig.json exists in the directory, well, if tsconfig.json exists.

How ? I didn't see that feature in the code.

Also, if you are targeting end developers who don't care about configurations, well, by definition, they don't care about how many configuration files in your scaffolding and you can go ahead to put more config.

Agreed, the number of configuration files doesn't matter. But the scaffolding for their project is theirs, I just provide a package to install with a single configuration file to get started (same idea as gulp : npm install gulp && touch gulpfile.js and you're good to get started)

@onigoetz
Copy link
Contributor Author

In the end, I think it's everybody has a different way to work, and when trying to configure ts-loader, that you are able to override compilerOptions but not set files within that configuration.

( Something like this :D )

Loader options tsconfig.json
compilerOptions
files This Pull Request

@johnnyreilly
Copy link
Member

johnnyreilly commented Nov 18, 2016

ts-loader will load typing files by default if tsconfig.json exists in the directory, well, if tsconfig.json exists.

How ? I didn't see that feature in the code.

It's not in the ts-loader code; it's how TypeScript itself behaves. Where a tsconfig.json file exists it denotes the root of a TypeScript project. Assuming files has not been directly specifed (i.e. it is omitted from the tsconfig.json) then all files within that directory are implicitly part of the TypeScript project context. If you take a look at most of the tsconfig.jsons in our test pack you'll see they are:

{
    "compilerOptions": {
    }
}

TBH I think it's totally valid for them to be

{
}

So given there are already sensible defaults in place it seems fairly reasonable to create an empty tsconfig.json and have done. i.e. Why add complexity to ts-loader if the alternative is having users create an empty json file?

Which reminds me; we should probably update the readme to simplify what's there. It hasn't been updated in ages.

@johnnyreilly
Copy link
Member

johnnyreilly commented Dec 5, 2016

I think what you've done here is super cool @onigoetz - I'm really impressed you implemented this, provided such a thorough PR and made your case really well.

But I don't think we want to merge this for the reasons that have been discussed in the thread so far.

I'm really sorry; I've thought long and hard about this. In the end I think the extra complexity that is introduced to support his simply isn't worth it given that the alternative is having an essentially empty tsconfig.json file sat in each project.

I hope this doesn't put you off contributing to ts-loader in the future.

@onigoetz
Copy link
Contributor Author

onigoetz commented Dec 5, 2016

Thanks for your answer, @johnnyreilly.

I understand completely your point, in the end, you would have to maintain it and it's completely fair to refuse a contribution :D

Of course I would have loved to see this merged, but we'll do it the way you described, it's not dramatic.

Don't worry, I will contribute if I have something to share ;-)

@onigoetz onigoetz closed this Dec 5, 2016
@johnnyreilly
Copy link
Member

Thanks @onigoetz - I appreciate that. 👍

@bigdrum
Copy link

bigdrum commented Jan 2, 2017

Just present a different use case for this feature that my team is having:

We have multiple webpack projects which share some common libraries and node_modules. We have no problem putting a single tsconfig to rule them all.

But without the ability to specify what files to include in webpack config, building one of these webpack projects requires compiling all files of all the projects as specified in the tsconfig. So it would be really nice if we could somehow override the files/include field in webpack just to optimize the build time.

Probably the feature to support our use case is less intrusive since we still keep the tsconfig.json.

@johnnyreilly
Copy link
Member

We always welcome PRs!

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.

5 participants