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

Resolve tsconfig.json extends path using node_modules resolution logic #18865

Closed
rtm opened this issue Sep 30, 2017 · 39 comments
Closed

Resolve tsconfig.json extends path using node_modules resolution logic #18865

rtm opened this issue Sep 30, 2017 · 39 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@rtm
Copy link

rtm commented Sep 30, 2017

I am trying to manage my tsconfigs by keeping them in a repo/subrepo, and was hoping this would work:

{
  "extends": "my-config-repo/tsconfig.standard",
   "compilerOptions": { }
 }

But I get

tsconfig.json(2,14): error TS18001: A path in an 'extends' option must be relative or rooted, but 'my-config-repo/tsconfig.standard' is not.

Is there some reason why the path given to extends, if neither relative nor absolute, could not be searched for using the node_modules lookup rules?

(I would also like multiple extends, but I suppose there was some reason for not doing that, and that would be another ticket anyway.)

@DanielRosenwasser DanielRosenwasser added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Sep 30, 2017
@aluanhaddad
Copy link
Contributor

Seems like a good idea. If this feature is added, I think it would be important that it also work with "paths".

Maybe this would be a small
step toward Project References 😉

@weswigham
Copy link
Member

weswigham commented Oct 1, 2017

We excluded it during the initial implementation due to complexity, but left an error for it in so we could go back and add it later if we needed to. An issue here is that the module resolution strategy we use is determined by your compiler options... and an extends option can be specifying just what that module resolution scheme is, which is kind of a circular dependency - you must know your module resolution scheme to resolve an extends option, but your extends option can lead to where you specify some module resolution scheme!

@aluanhaddad
Copy link
Contributor

@weswigham yeah that is paradoxical 😨. We find the source of truth only to learn from it that we were never meant to discover it...

@mightyiam
Copy link

How about escaping the paradoxical loop by just letting Node.js resolve this as it normally would?

BTW, for reference and inspiration, all of these support resolving this to a package export:

@mightyiam
Copy link

How good of an idea would it be to use "extends": "./node_modules/typescript-config-foo/index.json" until this is possibly implemented?

@weswigham
Copy link
Member

@mightyjam that's a relative path, so it's fine.

How about escaping the paradoxical loop by just letting Node.js resolve this as it normally would?

We do not always use node module resolution for your package (you have to specify in your configuration if you want node module resolution or a custom scheme - that's effectively what "moduleResolution": "classic" is), and it stands to reason that we'd use the same module resolution for your configuration as for your other files. Which leads to the issue that it becomes possible to not know what module resolution scheme to use to find your configuration until we have found your configuration, which is an issue.

@loucyx
Copy link

loucyx commented Oct 25, 2017

The solution can be something like setting the moduleResolution in the "children":

{
  "extends": "external-package/tsconfig",
  "compilerOptions": {
    "moduleResolution": "node"
  }
}

And, if the external-package/tsconfig.json file also have a moduleResolution, then you throw an error like: 'moduleResolution' can't be overwritten because 'external-package/tsconfig' is already setting a value. And, if not, then you resolve trough the node resolution.

This right now is a "blocker" for me, because I have a "core" module with the settings that will be shared among all my modules, and because the path is relative to node_modules, and npm has now a flat tree, this breaks.

For now I'm resolving it by adding setting the value to ../external-package/tsconfig instead of ./node_modules/external-package/tsconfig, but this solution is dirty because in development I need to have the core module one level above the module I'm working on in order to make it work.

TSLint already have this resolved, you can make something like the example above without any issues.

@mightyiam
Copy link

Yup. I'm the same use case as @lukeshiru.

I can't imagine why the node_modules being flat prevents you from resolving properly, @lukeshiru. I don't understand what you're doing there. Why do you resolve other than what my example shows?

tsconfig.json is not used after package is packaged, right? So my workaround should work.

@rtm
Copy link
Author

rtm commented Oct 26, 2017

How good of an idea would it be to use "extends": "./node_modules/typescript-config-foo/index.json" until this is possibly implemented?

Not a very good idea. In my mono-repo setup, the plan would be to have a config-like sub-repo, and it could end up getting put at a higher level, so you'd need to do ../node_modules/@myproject/config/tsconfig.json, essentially guessing where yarn would put things, which is exactly why I suggested some way to follow the built-in node module resolution algorithm.

@aluanhaddad
Copy link
Contributor

@lukeshiru there is always a --moduleResolution, but it is often implied.
--module commonjs implies --moduleResolution node, otherwise the language defaults to --moduleResolution classic.

@loucyx
Copy link

loucyx commented Oct 26, 2017

@mightyiam the problem is mentioned by @rtm. Let's say you have you children module using the parent's module config like this:

children-module/tsconfig

{
  "extends": "./node_modules/parent-module/tsconfig"
}

If you then, have that children module used somewhere else (let's calle it other-children-module), that reference becomes invalid, because instead of having this:

children-module/
└── node_modules/
    └── parend-module/

Now you have this:

other-children-module/
└── node_modules/
    ├── children-module/
    └── parent-module/

The flat dependency tree makes children and parent at the same level. So for that to work you need children-module to have the config changed to:

{
  "extends": "../parent-module/tsconfig"
}

The ideal scenario should be to solve it like TSLint does, by setting it like this:

{
  "extends": "parent-module/tsconfig"
}

And letting node resolve it.

@loucyx
Copy link

loucyx commented Oct 26, 2017

@aluanhaddad that still doesn't solve the issue. See my comment above for clarification.

@rtm
Copy link
Author

rtm commented Oct 26, 2017

Although by no means the only use case, the requirement might be simplest to understand by looking at the case where I want to install some config globally:

npm install --global cool-tsconfigs

Then use it by saying

"extends": "cool-tsconfigs/tsconfig.test.json"

@loucyx
Copy link

loucyx commented Oct 26, 2017

I don't agree with the use of global packages, I would be happy with tsconfig resolving extends like tslint does it, by using the npm module resolution (that's what I proposed first).

Right now my tslint files are beautiful:

{
  "extends": "@property/core-dev/tslint"
}

And my tsconfig are horrible :'( :

{
  // FIXME: Temporary fix until github.com/Microsoft/TypeScript/issues/18865 is fixed.
  // TODO: Change to @property/core/tsconfig when above is resolved.
  "extends": "../core/tsconfig",
  "compilerOptions": {
    "outDir": "./build"
  }
}

@rtm
Copy link
Author

rtm commented Oct 26, 2017 via email

@loucyx
Copy link

loucyx commented Oct 26, 2017

I don't think is a good practice to require a global package from a module. Quoting npm itself:

In general, the rule is:

  1. If you’re installing something that you want to use in your program, using require('whatever'), then install it locally, at the root of your project.
  2. If you’re installing something that you want to use in your shell, on the command line or something, install it globally, so that its binaries end up in your PATH environment variable.

@rtm
Copy link
Author

rtm commented Oct 26, 2017 via email

@spion
Copy link

spion commented Nov 14, 2017

We excluded it during the initial implementation due to complexity, but left an error for it in so we could go back and add it later if we needed to. An issue here is that the module resolution strategy we use is determined by your compiler options... and an extends option can be specifying just what that module resolution scheme is, which is kind of a circular dependency - you must know your module resolution scheme to resolve an extends option, but your extends option can lead to where you specify some module resolution scheme!

Only a problem if you don't specify the module resolution algorithm in that particular file. If you do, then extends should work and the final module resolution will not change since the final pkg config overrides it.

Of course that means you can't specify the module resolution in the base file. Personally, I think thats fine.

@ravenscar
Copy link

I understand this is a bit of a chicken-and-egg problem here but not being able to extend from another module without specifying a path seems really problematic, and makes the extension mechanism far less useful than it should be.

I can think of two trivial ways this could be done, the first resolving environment variables in the extends path, and the second allowing some kind of scheme in the extends path which indicates how to resolve the module. e.g.

{
  "extends": "$BASEDIR/tsconfig"
}

and

{
  "extends": "node:my-base-package"
}

currently I have a bunch of tsconfig.json.template files which are substituted via sed (sed 's@$BASEDIR@'"$LERNA_ROOT_PATH"'@' tsconfig.json.template > tsconfig.tmp.json) into tsconfig.tmp.json which has to be in the .gitignore.

I then build the project using tsc -p tsconfig.tmp.json.

This works for our lerna monorepo, but seems like a bunch of silly hoops to jump through.

@martijnthe
Copy link

@ravenscar I like the scheme idea!

@rtm
Copy link
Author

rtm commented May 23, 2018

@ravenscar Either of these looks good. However, I would point out that moduleResolution is a compilerOptions, which applies to the compilation process, and would not necessarily, and possibly should not, apply to the process by which TS reads in and processes its configurations. There is potentially no need to worry about any conflict between the compiler's module resolution setting and that applied by TS in resolving extends paths. I see no problem with the latter always using node resolution strategy. For instance, AFAIK tslint always uses node resolution semantics for its extends option.

If it is considered important to have both resolution strategies available for extends, then I would slightly prefer a new extendsResolution property at the top level of tsconfig.json instead of embedding this information in the extends path itself with the scheme-like node: notation. Or, allow extends: {path: "configrepo/tsconfig.json", resolution: "classic"}, with the default for extends.resolution hopefully being node.

@chordmemory
Copy link

chordmemory commented Jun 14, 2018

IMO better to only support node than support nothing at all.

It's almost 100% certain people will be developing in a node environment. To me it seems like people here are confusing compiler settings with development settings.

NPM and yarn don't offer custom module resolution for a reason.

There is literally no use case for using anything other than node module resolution to resolve a base TSCONFIG

@nevir
Copy link

nevir commented Jul 3, 2018

A use case for this; I have two libraries:

core-typescript: vends a baseline tsconfig.json for use in a bunch of projects (with things like lib, esModuleInterop, etc)

code-style: vends a style-oriented tsconfig.json that extends core-typescript's tsconfig (with things like strict, etc)

And I want application code to depend on code-style.

My current approach is for code-style to declare core-typescript as a peer dependency, to enforce that it is installed at the top level of an application's node_modules—and assumes that it will only ever be a top level dependency. Brittle :(

node_modules/
  core-typescript
    tsconfig.json
  code-style
    tsconfig.json // extends "../core-typescript/tsconfig.json"
tsconfig.json // extends "./node_modules/code-style/tsconfig.json"

@loucyx
Copy link

loucyx commented Jul 3, 2018

Yup, @nevir's use case is exactly what I need.

@mattdell
Copy link

mattdell commented Aug 22, 2018

Another vote for this. I just want to keep my tsconfig in one place in a lerna repo. Rather surprised to see there's no support already for this.

In my case I have multiple packages and those packages can either be built standalone or with lerna. If they are built as standalone then they will have node_modules in the package. If they are built using lerna the node_modules get hoisted into the parent directory. Therefore I cannot use the following solution

{
  "extends": "./node_modules/@foo/bar/tsconfig.json"
}

because it may also be here

{
  "extends": "../../node_modules/@foo/bar/tsconfig.json"
}

The ideal solution appears to be

{
  "extends": "@foo/bar/tsconfig.json"
}

Using node module resolution would fix this issue.

@felixfbecker
Copy link
Contributor

This just bit me because I had to extend a tsconfig.json in node_modules, that itself had to extend a tsconfig.json from a package in node_modules. But it can't, because ./node_modules/foo is now actually ../foo.

@loucyx
Copy link

loucyx commented Jan 21, 2019

Hey @weswigham! I think that PR didn't fixed this issue. I have a repo with settings and I'm trying to extend them like this:

// child-module/tsconfig.json
{
  "extends": "@org-name/settings-repo/tsconfig.json"
}

And instead of looking for it in the node_modules directory, it looks for it in the cwd, which is not the expected behavior. If I use this instead:

// child-module/tsconfig.json
{
  "extends": "./node_modules/@org-name/settings-repo/tsconfig.json"
}

It will break because that only works in this scenario:

child-module/
└── node_modules/
    └── @org-name/settings-repo/

But then if I use children-module it breaks in this scenario:

other-module/
└── node_modules/
    ├── child-module/
    └── @org-name/settings-repo/

Because is not anymore in child-module/node_modules, instead it is in children-module/... You are forgetting about plain structures...

@charrondev
Copy link

@weswigham Any chance this could be re-opened. As @lukeshiru points out this hasn't been properly implemented. The current implementation is not in line with the described Module resolution node in the typescript documentation.

@mightyiam
Copy link

@lukeshiru, @charrondev, please allow me to suggest that opening a new issue might help. You know how no-one likes reopening closed issues.

@rtm
Copy link
Author

rtm commented Jun 10, 2019

I am confused by this reported "bug" because I have been using this feature extensively since it was implemented and it works exactly as expected. Perhaps something else is wrong in your case. You may need to provide more information or even a sample repo.

@charrondev
Copy link

This definitely work properly for me at this point on 3.4 release in most scenarios.

I'll try to put together my reproduction case soon though. It's something like the following:

- ~/workspace/oss-core/
- ~/workspace/oss-core/tsconfig.json (extends `@vanilla/tsconfig/core.json`)
- ~/workspace/oss-core/plugins/proprietary-plugin (symlink to other directory).
- ~/workspace/proprietary-plugin
- ~/workspace/proprietary-plugin/tsconfig.json (extends `@vanilla/tsconfig/core.json`)

In this file structure I would expect the tsconfig extension to be capable of being discovered from when looked at through the symlinked file.

Instead oss-core/tsconfig.json works, but proprietary-plugin/tsconfig.json does not.

I think that the real file path is being resolved before evaluating the config file. I believe this is similar to the issue @lukeshiru is describing, but slightly different.

In any case I can open a separate issue and try to make a sample repo for my issue.

@TidyIQ
Copy link

TidyIQ commented Jun 19, 2019

I'm not sure how people are saying this works... My project structure is like so:

parent
  /project
    /tsconfig.js
    /node_modules
      /@package
        /tsconfig.js

And contents of `parent/project/tsconfig.json is:

{
  "extends": "@package/tsconfig.json"
}

Yet it fails as it's trying to extend from parent/node_modules/package/tsconfig.json instead of from parent/project/node_modules/package/tsconfig.json. Why is it looking for node_modules in the parent of the root instead of root itself?

@dietergeerts
Copy link

I also can confirm that this just doesn't work. Going through the code of that PR, I can only find out that the internal code is just over engineered and complicated for no apparent reason. Why is just doing require not an option here and just let node do his work?

@dietergeerts
Copy link

I also want to note that it doesn't work by just doing:

"extends": "@company/config-typescript"

Why am I trying this? Because I want to have an .js file where I can dynamically build the config depending on the project extending it.

@weswigham
Copy link
Member

We don't support any kind of .js based configuration. At all. You should just have a tsconfig.json in the package root, which, ofc, can't do anything dynamic.

@dietergeerts
Copy link

dietergeerts commented Oct 17, 2019

We don't support any kind of .js based configuration. At all. You should just have a tsconfig.json in the package root, which, ofc, can't do anything dynamic.

would such things be on the roadmap? As mono-repos are used more and more, it is useful to share config with the same includes following the globs that all projects inside the mono repo complies to and have dynamic things in it.

For example, we do this with our babel and nyc config, and that works like a charm.

@weswigham
Copy link
Member

Go open an issue - #30400 kinda tracked it but was closed by the author.

chadoh added a commit to nearprotocol/NEARStudio that referenced this issue Feb 7, 2020
the goal: when someone clones this repo and runs `yarn` to install
dependencies, all of the dependencies for the templates get installed as
well. Additionally, this is done in such a way that linting works for
these projects.

This is a little bit tricky! Each template has a file at
`assembly/tsconfig.json` which has an `extends` directive. This
`extends` points to `../node_modules/assemblyscript/[...]`. So the
installation of packages for each of these templates must be done in
such a way that this relative lookup does not break. That's what the
`nohoist` option does.

Note that typescript supports some version of module resolution, rather
than relative path, for the `extends` option. I tried to use this
briefly and ran into problems, whereas `nohoist` worked very quickly. It
seems like other people have had problems with `extends` pointing to
packages as well:
microsoft/TypeScript#18865 (comment)
chadoh added a commit to nearprotocol/NEARStudio that referenced this issue Feb 7, 2020
the goal: when someone clones this repo and runs `yarn` to install
dependencies, all of the dependencies for the templates get installed as
well. Additionally, this is done in such a way that linting works for
these projects.

This is a little bit tricky! Each template has a file at
`assembly/tsconfig.json` which has an `extends` directive. This
`extends` points to `../node_modules/assemblyscript/[...]`. So the
installation of packages for each of these templates must be done in
such a way that this relative lookup does not break. That's what the
`nohoist` option does.

Note that typescript supports some version of module resolution, rather
than relative path, for the `extends` option. I tried to use this
briefly and ran into problems, whereas `nohoist` worked very quickly. It
seems like other people have had problems with `extends` pointing to
packages as well:
microsoft/TypeScript#18865 (comment)

This also adds the `yarn.lock`, since we have no config preventing it,
and this is considered best practice
(https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/)
chadoh added a commit to nearprotocol/NEARStudio that referenced this issue Feb 7, 2020
the goal: when someone clones this repo and runs `yarn` to install
dependencies, all of the dependencies for the templates get installed as
well. Additionally, this is done in such a way that linting works for
these projects.

This is a little bit tricky! Each template has a file at
`assembly/tsconfig.json` which has an `extends` directive. This
`extends` points to `../node_modules/assemblyscript/[...]`. So the
installation of packages for each of these templates must be done in
such a way that this relative lookup does not break. That's what the
`nohoist` option does.

Note that typescript supports some version of module resolution, rather
than relative path, for the `extends` option. I tried to use this
briefly and ran into problems, whereas `nohoist` worked very quickly. It
seems like other people have had problems with `extends` pointing to
packages as well:
microsoft/TypeScript#18865 (comment)

This also adds the `yarn.lock`, since we have no config preventing it,
and this is considered best practice
(https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/)
@LinqLover
Copy link

I think we should have the same option for the include field as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests