-
Notifications
You must be signed in to change notification settings - Fork 889
TSLint won't pick up additional rules. #909
Comments
@cerebrl What version of TSLint are you using? I'd recommend trying The |
I have the same problem and I have created a repo where it manifests. I an using tslint 3.2.1. Here is my package.json: Here is my tslint.json: The "export-name-rule is not picked up. When I run npm run lint |
I think this is a bug with how TSLint handles paths - working on fixing it now. In the meantime, things will work correctly if you install TSLint globally and just run |
I think the reason it isn't working for folks using npm scripts is that the |
I got this to work with no issues on @janjarfalk 's repo. I had to change his script to :
The -r option picked up the extra rules and the ** glob (instead of the {pages,services} glob) picked up the .ts files. |
Two people now have tried to put tslint options within the rules.json file... which will not work. Perhaps clarifying this in the readme is a good idea. |
Actually @HamletDRC, it theoretically should work. There are some strange bugs right now with how relative paths are handled though. I've hopefully fixed that all in #912 :) |
I'd like to update this thread with the info that everything is now working. I removed the I can confirm that, at least for me, adding the Thanks to everyone for the assistance! |
@cerebrl @janjarfalk: We just released TSLint v3.2.2 which should fix this bug. If you specify a custom rules directory on the command line, the path is treated as relative to the current working directory (as it was before). However, if you specify a path in a I went through and tested out your repo with v3.2.2 @janjarfalk and found that running |
@jkillian: Thank you! But I think you introduced a bug. "rulesDirectory": ["node_modules/tslint-microsoft-contrib"]
// Works as expected. "rulesDirectory": ["tslint/rules"]
// Works as expected. "rulesDirectory": ["node_modules/tslint-microsoft-contrib", "tslint/rules"]
// Warnings from `tslint-microsoft-contrib` ONLY. "rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"]
// Warnings from `tslint/rules` ONLY. |
@janjarfalk Interesting, although I'm not convinced this is a bug. As soon as TSLint finds an invalid path, it throws an error and aborts. I'm not sure it seems worth it to me to do anything fancier than that... Do you think different behavior would be better? |
I think you misunderstood. Both "rulesDirectory": ["node_modules/tslint-microsoft-contrib"]
// If this result in warnings... "rulesDirectory": ["tslint/rules"]
// ...and this doesn't result in any warnings. "rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"]
// I still expect this to result in warnings, but it doesn't. I have updated this repo https://github.com/janjarfalk/ionic2-angular2-typescript-boilerplate.git with a setup that reproduces the problem running Case A - tslint-microsoft-contrib first:// tslint.json
{
"rulesDirectory": ["node_modules/tslint-microsoft-contrib", "tslint/rules"],
"rules": {
"missing-jsdoc": true, // A rule form Microsoft
"export-class-name": true // A rule from me (tslint/rules folder)
}
} // ACUTAL output
app/MyApp.ts
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts
// EXPECTED output
app/MyApp.ts
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts
app/pages/page1/page1.ts
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1
Case B - tslint/rules first:// tslint.json
{
"rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"],
"rules": {
"missing-jsdoc": true, // A rule form Microsoft
"export-class-name": true // A rule from me (tslint/rules folder)
}
} // ACTUAL output
app/pages/page1/page1.ts
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1
// EXPECTED output
app/MyApp.ts
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts
app/pages/page1/page1.ts
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1
|
Thanks for the detailed explanation @janjarfalk, I indeed did misunderstand you. This definitely seems like a bug, I'll look into it! |
That's a separate issue, I've filed #928 for it. |
@janjarfalk This should be fixed now in v3.3.0. As always, let us know if you come across any other issues 😃 |
@jkillian Awesome! Will do! Happy to help! |
It may help someone down the road: after reading the source, I learned that custom rules are loaded in the following way:
So |
@alexanderbird that should mostly be covered by the "Important Conventions" section in the README and docs site. open to PRs if you think the docs could be improved. |
@alexanderbird Thank you! I started by looking here https://github.com/mgechev/codelyzer and didn't see any instructions for naming conventions. As soon as I changed the name of my rule, it started working! Probably there is a doc update on both projects that are necessary. |
🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖 🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋 |
I originally reported this issue at Microsoft's additional TSLint rules, but maybe it's better reported here:
TSLint works fine, but it won't pick up the rules supplied by this module. Here's what I've done:
npm install tslint-microsoft-contrib --save-dev
tslint.json
added "rulesDirectory": "node_modules/tslint-microsoft-contrib" as a root property.package.json
, where I'm running this as a NPM script to thescript
property, I wrotetslint --rules-dir node_modules/tslint-microsoft-contrib index.js
tslint.json
file that this module provides within therules
property.The bottom line, I could not get this to work for the life of me using this within an NPM script. Is that what the problem is? Can you just not pick these rules up from with the context of running this within NPM? Any help would be appreciated. Thanks.
The text was updated successfully, but these errors were encountered: