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

'use strict' in all of the files #342

Closed
malept opened this issue Apr 23, 2016 · 12 comments
Closed

'use strict' in all of the files #342

malept opened this issue Apr 23, 2016 · 12 comments

Comments

@malept
Copy link
Member

malept commented Apr 23, 2016

Moving conversation from #341.

I am not against using 'use strict', but if we do so, I'd like to consider switching to using strict-standard instead of standard. Or possibly eslint + eslint-config-standard + some strict config (the reasoning being that it's more likely that a given editor will have eslint support available more readily than standard - this may just mean that we keep some form of standard for npm test, and just add an .eslintrc to the project).

See also:

@malept
Copy link
Member Author

malept commented Apr 24, 2016

CC: @feross (since there's an obvious connection there)

@feross
Copy link
Contributor

feross commented Apr 24, 2016

I've never personally had the need to use 'use strict' in my code. It doesn't provide much of any safety. If it's required for const, then it can be added to the top of all files while still using standard. standard just won't enforce it.

I personally don't think it's worth the overhead of maintaining a custom .eslintrc file just to enforce 'use strict'. That's after all the reason standard was selected in the first place :) But I'll defer to what others want here, of course.

@malept
Copy link
Member Author

malept commented Apr 24, 2016

I was thinking of using the custom .eslintrc just so I don't have to install standard globally. I already have eslint installed globally for other projects.

@feross
Copy link
Contributor

feross commented Apr 24, 2016

@malept Why do you need to install standard globally? You can just run npm test, right?

@malept
Copy link
Member Author

malept commented Apr 24, 2016

Editor integration is easier that way.

@feross
Copy link
Contributor

feross commented Apr 24, 2016

Ahh, you can also just add eslint-config-standard as a dependency to package.json, and add an .eslintrc:

{
  "extends": "standard"
}

@develar
Copy link
Contributor

develar commented Apr 27, 2016

strict-standard is too strict. It is not easy (and no sense, I think) to use it. Since we already have file .eslintrc, I just add required rules to it.

@develar
Copy link
Contributor

develar commented Apr 27, 2016

strict-standard: Strict Standard (https://github.com/denis-sokolov/strict-standard) 
  /Users/develar/Documents/electron-packager/cli.js:14:3: Unexpected console statement.
  /Users/develar/Documents/electron-packager/cli.js:15:3: Don't use process.exit(); throw an error instead.
  /Users/develar/Documents/electron-packager/cli.js:20:22: Unexpected console statement.
  /Users/develar/Documents/electron-packager/cli.js:21:10: Unexpected console statement.
  /Users/develar/Documents/electron-packager/cli.js:22:5: Don't use process.exit(); throw an error instead.
  /Users/develar/Documents/electron-packager/cli.js:25:28: Unexpected console statement.
  /Users/develar/Documents/electron-packager/cli.js:26:35: Unexpected console statement.
  /Users/develar/Documents/electron-packager/common.js:1:1: Use the global form of 'use strict'.
  /Users/develar/Documents/electron-packager/common.js:12:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:54:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:59:30: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/common.js:66:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:70:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:74:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:76:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/common.js:81:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/common.js:143:7: Unexpected console statement.
  /Users/develar/Documents/electron-packager/common.js:147:7: Unexpected console statement.
  /Users/develar/Documents/electron-packager/index.js:22:17: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/index.js:26:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:43:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:58:20: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/index.js:69:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:72:3: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:77:17: 'cb' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/index.js:80:17: 'cb' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/index.js:86:20: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/index.js:116:9: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:124:11: Unexpected console statement.
  /Users/develar/Documents/electron-packager/index.js:147:9: Expected a function expression.
  /Users/develar/Documents/electron-packager/index.js:156:17: Unexpected console statement.
  /Users/develar/Documents/electron-packager/index.js:169:13: Unexpected console statement.
  /Users/develar/Documents/electron-packager/index.js:200:60: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/linux.js:11:90: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/mac.js:10:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/mac.js:14:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/mac.js:29:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/mac.js:35:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/mac.js:46:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/mac.js:183:88: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/mac.js:184:9: Unexpected console statement.
  /Users/develar/Documents/electron-packager/mac.js:189:89: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/mac.js:192:15: Unexpected console statement.
  /Users/develar/Documents/electron-packager/mac.js:199:36: 'err' is already declared in the upper scope.
  /Users/develar/Documents/electron-packager/test/basic.js:1:1: Use the global form of 'use strict'.
  /Users/develar/Documents/electron-packager/test/basic.js:11:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:22:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:84:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:110:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:141:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:185:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:222:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:251:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:253:40: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/test/basic.js:294:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:336:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:361:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:386:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:430:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/basic.js:485:115: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:499:83: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:514:83: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:514:105: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:573:13: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:590:13: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/basic.js:607:13: Unexpected use of undefined.
  /Users/develar/Documents/electron-packager/test/darwin.js:1:1: Use the global form of 'use strict'.
  /Users/develar/Documents/electron-packager/test/fixtures/el-0374/main.js:7:1: Parsing error: Unexpected token app
  /Users/develar/Documents/electron-packager/test/index.js:13:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/index.js:16:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/index.js:19:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/index.js:22:5: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/index.js:26:3: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/mac.js:14:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:50:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:81:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:117:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:157:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:192:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:220:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:258:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:286:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:361:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:395:5: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/mac.js:529:21: Unexpected console statement.
  /Users/develar/Documents/electron-packager/test/multitarget.js:13:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/multitarget.js:29:38: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/test/multitarget.js:70:38: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/test/multitarget.js:98:38: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/test/multitarget.js:124:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/util.js:88:76: Unexpected comment inline with code.
  /Users/develar/Documents/electron-packager/test/win32.js:20:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/win32.js:55:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/win32.js:63:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/win32.js:71:1: Expected a function expression.
  /Users/develar/Documents/electron-packager/test/win32.js:79:1: Expected a function expression.

For me, most of these errors are ridiculous.

@malept
Copy link
Member Author

malept commented Apr 27, 2016

Unexpected comment inline with code.

I have no problem with this one.

'cb' is already declared in the upper scope.

This one is reasonable. We should avoid shadowed variables.

Use the global form of 'use strict'.

For some reason, I couldn't get this one to show up when I was messing with it last night.

The rest we should disable.

@develar
Copy link
Contributor

develar commented Apr 27, 2016

This one is reasonable. We should avoid shadowed variables.

This warning about function parameters. Not about local variables. Currently, we have callback hell. In my fork I use Promises. But anyway it is incorrect — it is ok and legal to shadow top-level function parameter. Real solution — use Promise/await cannot be done in "fix style" commit. Easy fix — just rename param — not acceptable, I think. Modern languages, like Kotlin, allows me to shadow top-level member.

BTW, I want to use strict exactly because I want to use let instead of var.

@develar
Copy link
Contributor

develar commented Apr 27, 2016

Expected a function expression.

It is the most annoying warning — you have to write "var foo = function() {". It is ridiculous. http://eslint.org/docs/rules/func-style#expression

@develar
Copy link
Contributor

develar commented Apr 27, 2016

The rest we should disable.

The problem that we cannot customize rules. As @feross suggested, in my upcoming PR I use eslint-config-standard.

develar referenced this issue in develar/electron-packager Apr 27, 2016
develar referenced this issue in develar/electron-packager May 15, 2016
(cherry picked from commit 02260d9)
malept pushed a commit that referenced this issue May 25, 2016
@malept malept added this to the 7.0.3 milestone Jun 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants