Skip to content

Support configuration inheritance #215

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

Merged
merged 4 commits into from
Nov 23, 2017
Merged

Conversation

aluanhaddad
Copy link
Contributor

Adds support for inheritance via the "extends" options in tsconfig.json` files.

Additional tests may be necessary.

Resolves #214

@aluanhaddad aluanhaddad mentioned this pull request Nov 23, 2017
@frankwallis
Copy link
Owner

This looks fantastic, thank you!

Couple of questions:-

  • Should this be a major version bump or minor?
  • Does it resolve filenames using systemjs resolution or just relative paths, ie can you use extends: 'my-build-tools/sharedtsconfig.json?

@aluanhaddad
Copy link
Contributor Author

Should this be a major version bump or minor?

I am not quite sure but, after thinking it over, it should probably be a major version bump since it could easily change the emit for existing setups.

Does it resolve filenames using systemjs resolution or just relative paths, ie can you use extends: 'my-build-tools/sharedtsconfig.json?

This is a really important question.

It currently uses the same resolution as the primary tsconfig.json files which means that build-tools/sharedtsconfig.json should be allowed but TypeScript itself does not allow this. It is an error for "extends" to specify a non-relative or non-rooted path. There is an ongoing discussion in microsoft/TypeScript#18865 about extending the behavior to support --moduleResolution node which would definitely complicate the matter.

I think we need to think this over. My instinct is to align with TypeScript's behavior, raising an error if the path is not relative or rooted, something this PR currently does not do.

I also need to add tests using physical files.

@frankwallis
Copy link
Owner

frankwallis commented Nov 23, 2017

Should this be a major version bump or minor?

I agree, I prefer to err on the side of caution.

Does it resolve filenames using systemjs resolution or just relative paths, ie can you use extends: 'my-build-tools/sharedtsconfig.json?

My opinion is that it should support systemjs resolution. While it does not align with typescript's behaviour it does not take anything away from the user because they can just use ./ to get the relative path. Also because systemjs jspm package locations include the version number it would make it difficult to update 'my-build-tools/sharedtsconfig.json' without also updating all of the projects which use it to have the right path containing the new version number. Looking at the code I think it will support this anyway.

@frankwallis frankwallis merged commit 250c7e5 into frankwallis:master Nov 23, 2017
@loucyx
Copy link

loucyx commented Nov 28, 2017

As @aluanhaddad mentioned in his comment, my current issue with extends is that is not behaving as expected on a node project. I have a @namespace/core module that contains a centralized tsconfig.json and tslint.json that I want to share with all my modules in @namespace. So from other packages I want to be able to just do this on tsconfig.json:

{
  "extends": "@namespace/core/tsconfig"
}

This is already solved and working as expected in tslint.json, but for tsconfig.json I currently have this:

{
  // Temporary fix until https://github.com/Microsoft/TypeScript/issues/18865 is fixed.
  "extends": "./node_nodules/@namespace/core/tsconfig",
  "compilerOptions": {
    "outDir": "./build"
  }
}

So extends needs to be relative, and I need to specify outDir because the value set in @namespace/core is relative to it, not to the extending tsconfig.json. This last issue is not that big of a deal, but the first one is, because when you have more than one level of inheritance, it breaks because of the plain dir structure of node_modules. Meaning:

In 1 level of inheritance it works:

core-dev/
└── node_modules/
    └── @namespace
        └── core

Because the relative path (./node_nodules/@namespace/core/tsconfig) keeps being valid. But in more than 1 level of inheritance, it breaks:

other-module/
└── node_modules/
    └── @namespace
        ├── core-dev
        └── core

This is because now core and core-dev are in the same level, so the relative path in core-dev (./node_nodules/@namespace/core/tsconfig) is not valid anymore, it needs to be ../core/tsconfig now in order to work.

As soon as this is fixed, the workflow will be great because the duplication of settings will be drastically reduced and centralized, and that will be awesome.

@frankwallis
Copy link
Owner

@lukeshiru I have made some changes in version 9.0.0 which I hope will resolve your issue.

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.

3 participants