Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Update for TSLint v4 #124

Merged
merged 1 commit into from
Dec 5, 2016
Merged

Update for TSLint v4 #124

merged 1 commit into from
Dec 5, 2016

Conversation

timkendrick
Copy link
Contributor

Thanks for linter-tslint! It doesn't seem to work with the latest version of tslint, so I've updated the source to work with TSLint v4.

It all seems to work fine for me – let me know if there's anything I can help with to get this merged!

@Arcanemagus
Copy link
Member

Looking over this now!

@@ -60,9 +60,9 @@ export default {
requireResolve(TSLINT_MODULE_NAME, { basedir },
(err, linterPath, pkg) => {
let linter;
if (!err && pkg && pkg.version.startsWith('3.')) {
if (!err && pkg && pkg.version.startsWith('4.')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this project can support loading tslint from a Project's folder, we should probably support as many different versions of tslint as we can, assuming it's not that much work. Since it seems that v3 -> v4 isn't that big of a change I'd prefer if both versions were supported, so just an else if here for v4.

@dsifford
Copy link

dsifford commented Dec 2, 2016

@Arcanemagus Anything I can do to help with this? I can finish this off if @timkendrick is busy.

@Arcanemagus
Copy link
Member

@dsifford let's give @timkendrick another 3 days (1 week total) to respond before redoing the work he's done here.

@dsifford
Copy link

dsifford commented Dec 2, 2016

@Arcanemagus Sure, no problem.

To clarify, I was just going to fork the PR and change that one line that you requested. @timkendrick's work looks great; no need to reinvent the wheel 😄

Not sure how GitHub would attribute his work back to him though going that route. Didn't think of that. Probably wise to wait. 👍

@Arcanemagus
Copy link
Member

Yea, that's the problem is I'd like the work attributed to him.

Also it's not just one line changed, the logic needs to be fixed throughout as currently this would only work for v4.

@dsifford
Copy link

dsifford commented Dec 2, 2016

@Arcanemagus Ah, alright.. Well I'll fork and fix for myself for now. I'll chime back in a few days if I don't see the update and we can go from there.

@timkendrick
Copy link
Contributor Author

Hi guys – just seen these now (busy week). Agree that staying backwards compatible is a good idea, I was just being lazy.

Feel free to go ahead and make those changes (I'm not fussed about getting the credit) – if you don't have time I'll be able to take a look at it tomorrow.

Cheers!

@dsifford
Copy link

dsifford commented Dec 2, 2016

@timkendrick Well that's very kind of you!

If you're planning on wrapping this up tomorrow, I'll humbly step back and let you do so so that you're credited for your hard work.

If something comes up tomorrow and you just can't get to it, chime back here and I'll get it done. I've got an unusual amount of free time this weekend so it shouldn't be much trouble on my end. 👍

Thanks for your work on this!

@timkendrick
Copy link
Contributor Author

Hi guys – I've added a shim for TSLint v3 interoperability. This can easily be removed in the future if you choose to stop supporting it.

Seems to work fine with both TSLint v3 and TSLint v4 projects, falling back to TSLint v4 if no local module is found.

Give me a shout if you find any problems!

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Arcanemagus Arcanemagus merged commit 170f0da into AtomLinter:master Dec 5, 2016
@Arcanemagus
Copy link
Member

Btw, as it seems @AtomLinter/linter-tslint isn't currently very active here, if you are interested in becoming a maintainer that would be great as I don't use this project myself.

@scttcper scttcper mentioned this pull request Dec 5, 2016
@timkendrick timkendrick deleted the tslint-4 branch December 5, 2016 19:52
@timkendrick
Copy link
Contributor Author

timkendrick commented Dec 5, 2016

I'm actually currently sizing up VS Code so I'm probably not the best choice of maintainer either… thanks for the help merging this though!

@aitboudad
Copy link

@Arcanemagus any chance to release a new version :)

@Arcanemagus
Copy link
Member

Finishing that up in the next few minutes 😉.

@dsifford
Copy link

dsifford commented Dec 5, 2016

If absolutely nobody else is interested, I'll help maintain. Lots on my plate though so I don't think I could take lead on it.

@Arcanemagus
Copy link
Member

Published in v0.12.0.

@aitboudad
Copy link

now I get the following issue:

Error: Cannot find module 'app-root-path' 
....
at .atom/packages/linter-tslint/lib/main.js:149:18

@Arcanemagus
Copy link
Member

That's where the call to TSLint's lint method happens, without the full stack trace I can't say for sure but it sounds like that's coming from TSLint itself.

@aitboudad
Copy link

I'm using https://github.com/mgechev/codelyzer, if I remove codelyzer rules it's working

Error: Cannot find module 'app-root-path'
Error: Cannot find module 'app-root-path'
    at Module._resolveFilename (module.js:455:15)
    at Function.Module._resolveFilename (/usr/share/atom-beta/resources/electron.asar/common/reset-search-paths.js:35:12)
    at Function.Module._load (module.js:403:25)
    at Module.require (module.js:483:17)
    at EventEmitter.<anonymous> (/usr/share/atom-beta/resources/electron.asar/browser/rpc-server.js:232:70)
    at emitTwo (events.js:106:13)
    at EventEmitter.emit (events.js:191:7)
    at WebContents.<anonymous> (/usr/share/atom-beta/resources/electron.asar/browser/api/web-contents.js:219:13)
    at emitTwo (events.js:106:13)
    at WebContents.emit (events.js:191:7)
    at metaToValue (/usr/share/atom-beta/resources/electron.asar/renderer/api/remote.js:210:13)
    at Object.exports.require (/usr/share/atom-beta/resources/electron.asar/renderer/api/remote.js:297:10)
    at Object.electron.remote.require (/usr/share/atom-beta/resources/app.asar/src/electron-shims.js:78:28)
    at resolve (/home/my_workspace/node_modules/app-root-path/lib/resolve.js:32:17)
    at module.exports (/home/my_workspace/node_modules/app-root-path/lib/app-root-path.js:6:20)
    at Object.<anonymous> (/home/my_workspace/node_modules/app-root-path/index.js:4:18)
    at Module._compile (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:103:30)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/home/my_workspace/node_modules/codelyzer/angular/config.js:25:12)
    at Module._compile (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:103:30)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/home/my_workspace/node_modules/codelyzer/angular/templates/templateParser.js:4:16)
    at Module._compile (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:103:30)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/home/my_workspace/node_modules/codelyzer/angular/ng2Walker.js:10:24)
    at Module._compile (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:103:30)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:50:27)
    at Object.<anonymous> (/home/my_workspace/node_modules/codelyzer/noInputRenameRule.js:9:19)
    at Module._compile (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:103:30)
    at Object.value [as .js] (/usr/share/atom-beta/resources/app.asar/src/compile-cache.js:216:21)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (/usr/share/atom-beta/resources/app.asar/src/native-compile-cache.js:50:27)
    at loadRule (/home/my_workspace/node_modules/tslint/lib/ruleLoader.js:107:26)
    at findRule (/home/my_workspace/node_modules/tslint/lib/ruleLoader.js:82:20)
    at Object.loadRules (/home/my_workspace/node_modules/tslint/lib/ruleLoader.js:33:24)
    at Linter.getEnabledRules (/home/my_workspace/node_modules/tslint/lib/linter.js:163:44)
    at Linter.lint (/home/my_workspace/node_modules/tslint/lib/linter.js:78:33)
    at /home/abdellatif/.atom/packages/linter-tslint/lib/main.js:149:18

@Arcanemagus
Copy link
Member

Can you file a new issue? This PR isn't really the place to discuss this. The stack trace definitely confirms my guess though that this is deep within the lint run and not part of this package.

In your new issue please indicate which version of tslint is in your project.

@aitboudad
Copy link

done #127

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants