Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

global tslint throws error if extends config option references local package #1171

Closed
janslow opened this issue Apr 26, 2016 · 5 comments · Fixed by #1172
Closed

global tslint throws error if extends config option references local package #1171

janslow opened this issue Apr 26, 2016 · 5 comments · Fixed by #1172

Comments

@janslow
Copy link
Contributor

janslow commented Apr 26, 2016

Bug Report

  • TSLint version: 3.8.1
  • TypeScript version: 1.8.10
  • Running TSLint via: (please choose one) CLI

TypeScript code being linted

tslint.json:

{
  "extends": "shared-tslint-config"
}

(where shared-tslint-config is a locally installed NPM package providing a tslint config)

Actual behavior

(when using a globally installed tslint)

$ tslint src/index.ts --config tslint.json

/usr/local/lib/node_modules/tslint/lib/configuration.js:122
            throw new Error(("Invalid \"extends\" configuration value - could not require \"" + relativeFilePath + "\". ") +
            ^

Error: Invalid "extends" configuration value - could not require "shared-tslint-config". Review the Node lookup algorithm (https://nodejs.org/api/modules.html#modules_all_together) for the approximate method TSLint uses to find the referenced configuration file.
    at resolveConfigurationPath (/usr/local/lib/node_modules/tslint/lib/configuration.js:122:19)
    at loadConfigurationFromPath (/usr/local/lib/node_modules/tslint/lib/configuration.js:101:38)
    at Object.findConfiguration (/usr/local/lib/node_modules/tslint/lib/configuration.js:45:12)
    at processFile (/usr/local/lib/node_modules/tslint/lib/tslint-cli.js:111:41)
    at Array.forEach (native)
    at Object.<anonymous> (/usr/local/lib/node_modules/tslint/lib/tslint-cli.js:128:41)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)

Expected behavior

tslint should resolve configuration file from locally installed package shared-tslint-config

@janslow janslow changed the title tslint throws error if extends config option is used with a locally installed package and absolute path to config file global tslint throws error if extends config option references local package Apr 26, 2016
@janslow
Copy link
Contributor Author

janslow commented Apr 26, 2016

I expect this is because, when run globally, require.resolve in resolveConfigurationPath() only has access to other global packages, not local ones.

@janslow
Copy link
Contributor Author

janslow commented Apr 26, 2016

There is a workaround of using a relative or absolute path to the config file to extend, instead of a module ID.

This obviously means you have to rely on the config file in the package not changing location in the package.

@jkillian
Copy link
Contributor

This is known behavior and I had originally thought it was okay to have things work like this, but I'm glad you made this issue for discussion. This issue is pretty confusing behavior for end users, and changing it would be a big improvement.

Perhaps the best solution here is to use a 3rd-party require/resolve library that lets you "start" from a different starting location? So modules could be resolved relative to the location of the config file and also as they are now.

@janslow
Copy link
Contributor Author

janslow commented Apr 26, 2016

A package like resolve should do the job.

I'm happy to create a pull request, if you're happy with this as a solution.

@jkillian
Copy link
Contributor

Sure, a PR would be great! Thanks @janslow

janslow added a commit to janslow/tslint that referenced this issue Apr 27, 2016
janslow added a commit to janslow/tslint that referenced this issue Apr 28, 2016
jkillian pushed a commit that referenced this issue Apr 29, 2016
…files (#1172)

* Add `resolve` dependency (and custom typings)

* Use resolve instead of require in resolveConfigurationPaths

Fixes #1171

* Simplify resolveConfigurationPath

`resolve` can handle relative and absolute links.

* path-is-absolute is no longer needed.

* Install test packages in subdirectory, so they aren't in the scope of the main tslint binary

* Run `npm install` in test config directory to install test packages.

* Fix code style issue

* Add dev dependency on npm for grunt run:installTestDeps task

* Revert "Add dev dependency on npm for grunt run:installTestDeps task"

This reverts commit fa0b947.

* Replace run:installTestDeps task with npm-command:test

* Fix lint errors in Gruntfile

* Resolve config file relative to the cwd if it can't be found relative to the parent config

* Make rules in tslint-test-custom-rules package valid rules (copied from noFailRule)

* Add CLI test with a relative extend config

* Add test config package

And install it as a dev dependency

* Add CLI test for extending a package which is installed in tslint.

* Make test packages private

* Fix entry point of test packages

* Simplify non-relative test package

* Add unit test for loadConfigurationFromPath with configs outside of the tslint require path.
@adidahiya adidahiya added this to the TSLint v3.9 milestone Apr 29, 2016
tomduncalf pushed a commit to tomduncalf/tslint that referenced this issue Jun 14, 2016
…files (palantir#1172)

* Add `resolve` dependency (and custom typings)

* Use resolve instead of require in resolveConfigurationPaths

Fixes palantir#1171

* Simplify resolveConfigurationPath

`resolve` can handle relative and absolute links.

* path-is-absolute is no longer needed.

* Install test packages in subdirectory, so they aren't in the scope of the main tslint binary

* Run `npm install` in test config directory to install test packages.

* Fix code style issue

* Add dev dependency on npm for grunt run:installTestDeps task

* Revert "Add dev dependency on npm for grunt run:installTestDeps task"

This reverts commit fa0b947.

* Replace run:installTestDeps task with npm-command:test

* Fix lint errors in Gruntfile

* Resolve config file relative to the cwd if it can't be found relative to the parent config

* Make rules in tslint-test-custom-rules package valid rules (copied from noFailRule)

* Add CLI test with a relative extend config

* Add test config package

And install it as a dev dependency

* Add CLI test for extending a package which is installed in tslint.

* Make test packages private

* Fix entry point of test packages

* Simplify non-relative test package

* Add unit test for loadConfigurationFromPath with configs outside of the tslint require path.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants