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

Configuration Inheritance #9876

Closed
weswigham opened this issue Jul 21, 2016 · 36 comments
Closed

Configuration Inheritance #9876

weswigham opened this issue Jul 21, 2016 · 36 comments
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript

Comments

@weswigham
Copy link
Member

weswigham commented Jul 21, 2016

Our friends from @angular asked for this feature while they were visiting last week, since they use a lot of configuration files internally.

Proposal

Add a new top-level (alongside compilerOptions, files, includes, and excludes) tsconfig parameter - extends (to steal the term from eslint).

  • Its value is a string or an array of strings which are interpreted as rooted or relative paths (so starting with either a drive root or . or ..) from the current tsconfig to other tsconfigs to inherit from - with or without a .json extension, if no extension is provided, we first look for an extensionless file by the name, and failing that, add a .json extension and look for that.
  • If we add recommended or built-in configs to inherit from in the future, those will look like a module identifier, rather than a path (ie, "typescript/strict"). For now, non-relative, non-rooted paths will be an error. At present, built-in or shareable configs are not part of this proposal.
  • All configs your config inherits from are first parsed and resolved (thereby filling in anything they inherit), and then merged. Configs later in the list override the configuration provided by those earlier in the list. Configuration in the active config can override those provided by an inheriting config. If a circularity is encountered, we report an error.
  • If any inherited configs we load have errors, we will report them.
  • Top-level members other than compilerOptions are completely overridden by following definitions. compilerOptions at the top level is merged (though its individual keys will still be overridden as though they were top-level keys).
  • Relative paths found in the configuration file will be resolved relative to the configuration file they originated in.

Examples

configs/base.json:

{
  "compilerOptions": {
    "allowJs": true,
    "noImplicitAny": true,
    "strictNullChecks": true
  }
}

configs/tests.json:

{
  "compilerOptions": {
    "preserveConstEnums": true,
    "stripComments": false,
    "sourceMaps": true
  },
  "exclude": [
    "../tests/baselines",
    "../tests/scenarios"
  ],
  "include": [
    "../tests/**/*.ts"
  ]
}

tsconfig.json:

{
  "extends": "./configs/base",
  "files": [
    "main.ts",
    "supplemental.ts"
  ]
}

tsconfig.nostrictnull.json:

{
  "extends": "./tsconfig"
  "compilerOptions": {
    "strictNullChecks": false
  }
}

tsconfig.tests.json:

{
  "extends": ["./configs/tests", "./tsconfig"],
  "compilerOptions": {
    "module": "commonjs"
  }
}

tsconfig.tests.browser.json:

{
  "extends": ["./configs/tests", "./tsconfig"],
  "compilerOptions": {
    "module": "amd"
  }
}

@alexeagle @DanielRosenwasser Does this cover your usecases?

@kitsonk
Copy link
Contributor

kitsonk commented Jul 21, 2016

Not to take anything away from the @angular team 😄, but other projects would benefit from this too. Currently @dojo is managing this via the tooling tasks which overwrite configuration "magically" when doing different targets, but it would be great to just provide this as a .json file that tsc understood inherently.

@weswigham
Copy link
Member Author

weswigham commented Jul 21, 2016

@kitsonk We actually started doing the same thing with our debug/release build duality inside the compiler, if you look at our gulpfile. 😆

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jul 21, 2016
@alexeagle
Copy link
Contributor

SGTM cc @IgorMinar

I understand about file references being rooted relative to the file requested by -p option, though that is clearly the thorny issue here, as knowledge of resolution is non-local in the tsconfig containing the reference. If tsconfig.json extends ./a.json and subdir/tsconfig.json extends ../a.json there are no correct options for relative paths in a.json?

@weswigham
Copy link
Member Author

weswigham commented Jul 21, 2016

@alexeagle Correct. Though, you could still have useful things such as
"exclude": ["node_modules"], which is probably still what you wanted
regardless of where its resolved from. My general recommendation would be
to avoid using paths in inheritable configurations unless you're really
sure you want it. Generally, the config extends member will behave mostly
as Object.assign({}, ...extendedConfigs). Or at least that's the intent.
Rewriting all the paths complicates that a lot... And requires knowledge of
what is a path and what isn't.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

the config extends member will behave mostly as Object.assign({}, ...extendedConfigs

You could still do this if you say you are merging the "resolved" version of the config files. so you first look at the config file, and make all the file path properties absolute using the containing file location, something we already do today, then merge the result.

@alexeagle
Copy link
Contributor

+1 for resolving the config before merging. Tools will also have an
incredibly hard time with paths in an included file that point to the wrong
place, even if it looks canonical like "node_modules" I don't think the IDE
should be expected to look in parent directories for that.

On Thu, Jul 21, 2016 at 3:53 PM Mohamed Hegazy notifications@github.com
wrote:

the config extends member will behave mostly as Object.assign({},
...extendedConfigs

You could still do this if you say you are merging the "resolved" version
of the config files. so you first look at the config file, and make all the
file path properties absolute using the containing file location, something
we already do today, then merge the result.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9876 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5I98tT3BljBILqW3if1QgQHxljrYxks5qX_hrgaJpZM4JSL8V
.

@weswigham
Copy link
Member Author

weswigham commented Jul 21, 2016

@mhegazy We could do that for the top-level options easily, but for things within the compilerOptions hash, it seems like a really dicey proposition. I originally wanted to not resolve anything just because I don't think we can identify every potential path in the compilerOptions hash (specifically, paths which appear in 'object' or 'list' argument kinds). So I thought differing behavior between options would be surprising, making not resolving paths at all the least surprising option.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

it is very hard to reason about relative paths when consumed in the context of a different file.

we already mark some compiler options as isFilePath and make these absolute relative to the tsconfig.json path. so this would not be too much different here. we just have to resolve them before we do the merge.

@weswigham
Copy link
Member Author

weswigham commented Jul 21, 2016

@mhegazy But we don't currently mark members of object compiler options as paths (like the paths option). Also, in the extensibility model pr, I have no way of knowing if an extension's arguments are a path.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

"paths" is not. it is always relative to baseURL, baseURL however is a file path.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

for the extensions, this is a hard one. no idea what we can do there. these are opaque objects and we have no visibility in them.

@weswigham
Copy link
Member Author

@mhegazy since paths isn't an issue, I guess we can just resolve all filepaths we see in the config and say that the extensions hash is treated as an opaque object, so you shouldn't rely on inheriting paths in it anyway?

Or maybe I should just not think about extensions, solve this for this, then rebuild the extensions argument structure on top of it? That sounds better, I think.

In any case, I'll update the OP to reflect that we will resolve paths in all path-based options.

@clavecoder
Copy link

This is certainly more versatile than my suggestion (##9835), yet, perhaps you would like to check with the angular team if the environment based solution (which avoids the relative paths issue) is good enough. I know they source multiple packages from one repo that each have their own tsconfig.json files, so maybe that is their driver. It would be good to add a user story to understand what problems are addressed by this solution.

I'm trying to follow the Angular 2 pattern my repo and figured I was doomed to copy settings everywhere. Multiplackage development it's so common that there is a popular NodeJS tool to support it: https://lernajs.io/

@kitsonk
Copy link
Contributor

kitsonk commented Jul 22, 2016

perhaps you would like to check with the angular team if the environment based solution (which avoids the relative paths issue) is good enough

@alexeagle is the Angular team member interfacing with the TypeScript team... 😉

@alexeagle
Copy link
Contributor

IMO the biggest problem to overcome in this change is for editors to catch up. We try to keep all the editors working which means we can't update to a new tsconfig.json feature for a long time after it's been added.
I don't think that reading from the environment is useful for us, we don't want to parameterize the build based on a user preference or runtime switch, we just want to have multiple packages share some settings.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jul 24, 2016

I think this would be helpful for generating paths programmatically. However if only the top level properties of compilerOptions are merged then that wouldn't work. I'd like to see array properties merged or at least concatenated.

@alexeagle
Copy link
Contributor

How would you use this to generate paths? Do you mean you'd write a
program that creates tsconfig.json files? Or do you want TypeScript to
somehow fill in paths for you automatically from relative locations of
the files?

On Sat, Jul 23, 2016 at 7:54 PM Aluan Haddad notifications@github.com
wrote:

I think this would be helpful for generating paths programmatically.
However if only the top level properties of compilerOptions are merged
then that wouldn't work. I'd like to see array properties merged.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9876 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5I_xbKEUuaWpTwQktxlqXSnei4ltVks5qYtPHgaJpZM4JSL8V
.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jul 24, 2016

Do you mean you'd write a
program that creates tsconfig.json files?

Yes exactly. For example, it might run as a post install hook.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 24, 2016

Just curious why u need this, is this for jspm modules?

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jul 25, 2016

@mhegazy Thank you for asking. Yes this is exactly what I am trying to do.

I am working on some tools that parses jspm.config.js files to automatically install correctly versioned declaration files and map them to the package scoped import names via tsconfig.json.

The plan is to use the information to determine the .d.ts files corresponding to the semver tags of actually installed version, retrieve them, and install them as non ambient external modules (multi-version support, no typings style ambient declare).

The only way I can think to do this, is to automatically generate paths.

Are there are any plans for TypeScript to support jspm in the near future? I saw that the #2233 was recently closed.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 25, 2016

The correct fix here is for TS to add module resolution support for jspm modules. The concerns we had were the relative instability of the framework, the lack of documentation and the runtime configuration structure that is not friendly to static analyzers. I think w need to reasses this for TS2.1; but do not think the approach of auto generating path mapping is sustainable.

@weswigham
Copy link
Member Author

I've opened #9941 with an implementation of the proposal thus far - it didn't seem like there was any contention over anything other than path handling. That question was resolved, so it seemed as though consensus had been reached, at least for now.

@weswigham weswigham self-assigned this Jul 26, 2016
@aluanhaddad
Copy link
Contributor

@mhegazy I'm glad to know this is still under consideration and I really hope it gets implemented. Thank you for your feedback regarding the sustainability of auto generating path mapping.

@wclr
Copy link

wclr commented Aug 25, 2016

Why not support "tsconfig.js" in standard node format?
Though hope this feature with "extend" will land soon.

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Sep 13, 2016
@mhegazy mhegazy added this to the TypeScript 2.1 milestone Sep 13, 2016
@mhegazy mhegazy removed the Breaking Change Would introduce errors in existing code label Sep 13, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 13, 2016

This should be available in the nightly builds starting 9/14/2016

@tomitrescak
Copy link

Shall this work with @next? Running 2.1.0-dev.20161004.

In my src/tsconfig.json I keep the global project configuration with es6 modules
In my src/server/tsconfig.json I have following configuration

{
  "extends": "../tsconfig",
  "module": "commonjs"
}

Yet, when I compile my project I still have es6 modules everywhere :/

@kitsonk
Copy link
Contributor

kitsonk commented Oct 4, 2016

It isn't going to magically know what files are included and excluded, just based on its location and you would need to invoke the compiler twice.

So in src/tsconfig.json you need to exclude src/server/ and in src/server/tsconfig.json you need to limit to only src/server/ and invoke tsc twice.

@tomitrescak
Copy link

tomitrescak commented Oct 4, 2016

Hmm, I'm not sure if I follow correctly. These are my two typings.jsons

/tsconfig.json

{
  "compileOnSave": true,
  "compilerOptions": {
    "target": "es6",
    "module": "es6",
  },
  "include": [
    "./server/**/*.ts",
    "./client/**/*.ts",
    "./client/**/*.tsx",
    "./typings/**/*.ts",
    "./shared/**/*.ts"
  ],
  "exclude": [
    "node_modules",
    "./server/**/*.ts"
  ]
}

and the /server/tsconfig.json

{
  "extends": "../tsconfig.json",
  "module": "commonjs",
  "include": ["./**/*.ts"]
}

Now the server files are not emitted at all.

@kitsonk
Copy link
Contributor

kitsonk commented Oct 4, 2016

Where did your extends go in /server/tsconfig.json? How are you invoking the compiler?

@tomitrescak
Copy link

Hi, I updated the above example. I simply executed tsc twice in the same destination.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 4, 2016

The file that extends a configuration should be a valid configuration file itself, i.e.:

{
  "extends": "../tsconfig.json",
  "compilerOptions": {
      "module": "commonjs"
   }
}

@weswigham
Copy link
Member Author

@Aleksey-Bykov Alright. Done; but I don't think closed issues are normally considered a canonical source of documentation. 😛

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 16, 2016

The correct fix here is for TS to add module resolution support for jspm modules. The concerns we had were the relative instability of the framework, the lack of documentation and the runtime configuration structure that is not friendly to static analyzers. I think w need to reasses this for TS2.1; but do not think the approach of auto generating path mapping is sustainable.

@mhegazy Sorry for resurrecting this thread. In spite of your advice, which I still think is correct advice, I heedlessly went ahead with an experiment. I found that analyzing the jspm.confg.js could be used to install correct types and that multiple versions could be supported, but my solution only works with Aurelia, and will fail if they restructure their git repository. However generating paths was surprisingly pleasant, especially since I got to write a node.js app using TypeScript's async/await for the first time. From the experience, I would say that analyzing where to get the declaration files is the difficult part.
Thanks for your time.

@evil-shrike
Copy link

In the original proposal it was suggested to allow using arrays as extends values.
But it seems it wasn't implemented (only strings are allowed).

Here's our use case where it's needed to have multiple base config (array in extends).

Given an app written in TS. I have several dependencies installed via bower into vendor_components folder. The app have its own tsconfig.json which is very simple as it's based on other configs:

    "extends": [
      "./vendor_components/ourFramework/tsconfig", 
      "./vendor_components/ourModule/tsconfig"
    ],
    "include": [
        "src/**/*.ts",
        "src/**/.*.ts",
        "vendor_components/ourFramework/src/**/*.ts",
        "vendor_components/ourFramework/src/**/.*.ts",
        "vendor_components/ourModule/src/**/*.ts",
        "vendor_components/ourModule/src/**/.*.ts"
    ]

The config "./vendor_components/ourFramework/tsconfig" is major, it describes all compilerOptions. But ourModule's config is also needed as it contains paths,
like:

"lib/*": [ "src/lib/*" ],

So it's ok if a lib/fw in vendor_components contains its own tsconfig. If the app want include that lib/fw sources it should use that tsconfig as well. But currently it can use only one. Other's config have to replicated in app's tsconfig.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

10 participants