search

ricardofbarros / linter-js-standard

Atom linter plugin for JavaScript, using JavaScript Standard Style
https://atom.io/packages/linter-js-standard
MIT License
99 stars 48 forks source link

Object.dirname is deprecated. #234

Closed Emrio closed 5 years ago

Emrio commented 5 years ago

Hello, I just got this weird warning from "Deprecation Cop" on Atom. I don't know how to replicate, and I don't know either why it poped :/ My Atom installation :

Atom    : 1.40.1
Electron: 3.1.10
Chrome  : 66.0.3359.181
Node    : 10.2.0

My (enabled) packages : 

Community Packages (13) /Users/emrio/.atom/packages
├── atom-discord@2.0.2
├── atom-easy-jsdoc@4.12.5
├── atom-ide-ui@0.13.0
├── atom-ternjs@0.20.0
├── atom-typescript@13.3.0
├── build-oasis@0.1.1
├── busy-signal@2.0.1
├── file-icons@2.1.35
├── hyperclick@0.1.5
├── intentions@1.1.5
├── language-ocaml@1.9.5
├── linter-js-standard@7.0.0
└── linter-ui-default@1.8.0

The error + stacktrace : Argument to path.dirname must be a string

Object.dirname (/Applications/Atom.app/Contents/Resources/app.asar/src/electron-shims.js:9:10)
Object.lint (/Users/emrio/.atom/packages/linter-js-standard/lib/linter-js-standard.js:52:19)
a.factory (/Users/emrio/.atom/packages/atom-ide-ui/modules/atom-ide-ui/pkg/atom-ide-diagnostics/lib/services/LinterAdapter.js:309:42)
a._callFactory (/Users/emrio/.atom/packages/atom-ide-ui/node_modules/rxjs/bundles/Rx.min.js:107:23)
a.tryDefer (/Users/emrio/.atom/packages/atom-ide-ui/node_modules/rxjs/bundles/Rx.min.js:106:442)
new a (/Users/emrio/.atom/packages/atom-ide-ui/node_modules/rxjs/bundles/Rx.min.js:106:383)
sonicdoe commented 5 years ago

Looking at your installed packages, you don’t seem to have linter installed which is required for linter-js-standard to work. Do you get a prompt which asks you to install this dependency?

Emrio commented 5 years ago

Oh, I deactivated it because I thought it was a standalone package I installed from when I was looking for a linting tool. I have just reenabled linter; it prompted me a message telling me linter and atom-ide-diagnostics were conflicting because they both show linting errors, so I think this is why it still worked.

sonicdoe commented 5 years ago

You’re right, I missed atom-ide-ui in the list. This could certainly contribute to the error, however, since Facebook retired the Atom IDE, I don’t think adding support for it is warranted.

Arcanemagus commented 5 years ago

You'll need to add a check that textEditor.getPath() actually returned a path before this line: https://github.com/ricardofbarros/linter-js-standard/blob/c672ab904fcb9b1a6ba6512a109a8138a64b3ffd/lib/linter-js-standard.js#L52

The linter API currently requires a path in the messages to associate them with the editor, so although several linters can run on raw text with no actual file associated with it, you can't report the messages back.

_Back in Node.js v6 path.dirname was changed to require a value for the path to check, before that it would happily accept an undefined value._

sonicdoe commented 5 years ago

Would linter call this specific linter if there’s no path? Looking at linter’s isPathIgnored(), I was under the impression that it would always ignore “files” without a path.

Arcanemagus commented 5 years ago

It shouldn't, but it's a common problem in many providers that they get called on ones that don't return a path. I think it's an issue during session restores were part of the system thinks the editor has a path and another part doesn't? I haven't tracked down the issue myself, but it happens with both linter and atom-ide-ui.

Emrio commented 5 years ago

I close this issue because a warning message didn't pop up again and it might have been thrown by atom-ide-ui.

sonicdoe commented 5 years ago

Thank you for the explanation, @Arcanemagus. I’ve now added a check in https://github.com/ricardofbarros/linter-js-standard/commit/7780e7cede7a277bbe3c886b51477bd022a86b9a.