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

defaulting to tsconfig: true #198

Open
aluanhaddad opened this issue Feb 27, 2017 · 6 comments
Open

defaulting to tsconfig: true #198

aluanhaddad opened this issue Feb 27, 2017 · 6 comments

Comments

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Feb 27, 2017

With the breaking change of removing TypeChecking, I thought it would be a good time to propose something that I have wanted for a long time.

When setting up a new project, I will always have a tsconfig.json file for IDE support. But it is ever so slightly annoying to have to add the "typescriptOptions.tsconfig": true to jspm.config.js/systemjs.config.js every time I set up this plugin.

Would it be possible to assume this by default going forward?

Given that users will now need to rely on type checking in their IDE, it is more essential than ever that transpilation settings that the syntactic transformation options, some of which block the user of certain features, are in sync between one's IDE and plugin-typescript.

Another thing is that sometimes people use both this and plugin-babel on the same chunks of code to verify consistency of certain transpiler behavior.

I think this would strongly benefit new users as this is a gotcha that I frequently have to remind people about and explain how to configure.

Thank you.

@frankwallis
Copy link
Owner

I am happy to change this, but how would you expect it to work in the situation where you have file-level typescriptOptions as well as global typescriptOptions defined? Currently the plugin will resolve both tsconfig files if specified and merge all the configuration (see https://github.com/frankwallis/plugin-typescript/blob/master/src/resolve-options.ts#L17) although I'm not sure that is the correct behaviour.

@aluanhaddad
Copy link
Contributor Author

Thanks for considering this.
Personally, I do not think merging file-level typescriptOptions with global settings is a great idea in general, unless it is unavoidable. The extends option that tsconfig.json supports would ideally control configuration merging and without specifying it, I would prefer as much isolation as possible.

That said I would say that any top level tsconfig.json file should be merged with the global typescriptOptions and since global options are currently merged into file specific typescriptOptions that behavior probably should work with this change, thusly merging all of the options.

I may have misunderstood the finer points of your statement but in general I think of wanting file specific settings more to disable behavior than to enable it.

@aluanhaddad
Copy link
Contributor Author

aluanhaddad commented Mar 6, 2017

I've been giving this some thought and I think the current merging behavior is the right approach, at least for the time being.

So for give the following directory layout

src\
config.js
tsconfig.json

the settings from the ambient tsconfig.json file would be merged into any top level "typescriptOptions": {...} in config.js unless those options specify "tsconfig": false.

For nested configs

src\
config.js
tsconfig.json
src\primary\main.ts
src\primary\tsconfig.json

the settings from the ambient src\primary\tsconfig.json file would be merged into any

"packages:" { 
  "src\primary": {
    "meta": {
      "main.ts": {
        "typescriptOptions": { ... }
      }
    }
  }  
}

in config.js unless those options specify "tsconfig": false.

Does this seems correct?

@aluanhaddad
Copy link
Contributor Author

@frankwallis would you be open to a pull request for this? I'm a little on the fence about whether this belongs in plugin-typescript or JSPM.

On the one hand, introducing the behavior here will have the broadest impact on users, but on the other, JSPM users are more comfortable with configuring transpiler options, but there is a discoverability issue.

With the removal of type checking support, the most important use case is changing the module and target.

Another way to address this would be to warn users when a tsconfig.json is present but is not being used by typescriptOptions (whether to opt-out or to opt in).

The primary interest I have here is to make it easy for the IDE experience and the runtime behavior to correspond.

If you feel this is more of a JSPM feature, then I would happily raise the issue there, with the suggestion that a pre-populated options object be added to the default jspm.config.js similarly to how it is added for plugin-babel.

Another thing that occurs to me is that there should perhaps be a single line of log output that indicates when a tsconfig file is in fact being used. I'll open at separate issue for that.

Thank you.

@frankwallis
Copy link
Owner

Yes, I am open to a pull request but I would like to know what the impact will be on existing users. I think that all tsconfig handling would be better handled in this plugin than in jspm.

@aluanhaddad
Copy link
Contributor Author

I think the simplest thing to do would be to apply a top-level tsconfig only but _not _ to merge it with any meta.typescriptOptions if specified.

Part of the reason I thought to put this in JSPM was that the generated configuration would make it explicit that the tsconfig file was in use.

I agree though that it belongs here.

I think simply logging a message stating that it's been picked up, possibly along with some other information such as the TypeScript version, could be a good way to go. Both of Webpack's TypeScript loaders do this (print using tsconfig and TypeScript version) for what it's worth.

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

No branches or pull requests

2 participants