Add eslint #477
Add eslint #477
Conversation
| @@ -251,9 +252,6 @@ | |||
|
|
|||
| function JsonToCsv(_input, _config) | |||
| { | |||
| var _output = ''; | |||
| var _fields = []; | |||
There was a problem hiding this comment.
These were never used.
| ? _input.fields | ||
| : objectKeys(_input.data[0]); | ||
| ? _input.fields | ||
| : objectKeys(_input.data[0]); |
There was a problem hiding this comment.
We can relax the rules if we're not concerned with indentation like this. But it was easier to just fix this case.
| @@ -644,8 +642,8 @@ | |||
| { | |||
| var contentRange = xhr.getResponseHeader('Content-Range'); | |||
| if (contentRange === null) { // no content range, then finish! | |||
| return -1; | |||
| } | |||
| return -1; | |||
There was a problem hiding this comment.
Good example of catching stuff that's completely wrong.
| var remaining; | ||
| this.stream = function(s) | ||
| { | ||
| string = s; |
There was a problem hiding this comment.
This was never read.
| @@ -1238,7 +1235,7 @@ | |||
|
|
|||
| var nextDelim = input.indexOf(delim, cursor); | |||
| var nextNewline = input.indexOf(newline, cursor); | |||
| var quoteCharRegex = new RegExp(escapeChar.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, '\\$&')+quoteChar, 'g'); | |||
| var quoteCharRegex = new RegExp(escapeChar.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&')+quoteChar, 'g'); | |||
There was a problem hiding this comment.
We can relax https://eslint.org/docs/rules/no-useless-escape, but I think the resulting regex is easier to read.
| @@ -1401,17 +1398,17 @@ | |||
| /** | |||
| * checks if there are extra spaces after closing quote and given index without any text | |||
| * if Yes, returns the number of spaces | |||
| */ | |||
| function extraSpaces(index) { | |||
| var spaceLength = 0; | |||
There was a problem hiding this comment.
Mixed tabs and spaces
player/player.js
Outdated
| var stepped = 0, chunks = 0, rows = 0; | ||
| var start, end; | ||
| var parser; | ||
| var parser; // eslint-disable-line no-unused-vars |
There was a problem hiding this comment.
Not sure if this is used as part of player.html so I kept it in.
There was a problem hiding this comment.
I will prefer to keep player as is and ignore it.
| @@ -1,64 +1,61 @@ | |||
| (function() { | |||
| "use strict"; | |||
There was a problem hiding this comment.
I removed the IIFE wrapper. Didn't seem necessary in a node-only context and it caused eslint to flag the use of sync node APIs at the "top level" (since IIFE is not the top level).
| @@ -825,7 +825,7 @@ var PARSE_TESTS = [ | |||
| input: 'A,B,C\r\n1,2.2,1e3,5.5\r\n-4,-4.5,-4e-5', | |||
| config: { header: true, dynamicTyping: { A: true, C: true, __parsed_extra: true } }, | |||
| expected: { | |||
| data: [{"A": 1, "B": "2.2", "C": 1000, "__parsed_extra": [ 5.5 ]}, {"A": -4, "B": "-4.5", "C": -0.00004}], | |||
| data: [{"A": 1, "B": "2.2", "C": 1000, "__parsed_extra": [5.5]}, {"A": -4, "B": "-4.5", "C": -0.00004}], | |||
There was a problem hiding this comment.
We can relax stylistic stuff like this.
|
Tests are failing since eslint doesn't support Node.js 0.12. Since that reached end of life at the end of 2016, I think we should just drop support for that. I'll open a separate PR for that. Otherwise, I can switch to an older eslint version. |
|
I've merged the drop of 0.12, you should rebase to make the test pass. |
|
@pokoli Thanks. Tests are passing now. |
package.json
Outdated
| @@ -47,9 +48,10 @@ | |||
| "serve-static": "^1.7.1" | |||
| }, | |||
| "scripts": { | |||
| "lint": "eslint --no-ignore papaparse.js Gruntfile.js .eslintrc.js 'player/**/*.js' 'tests/**/*.js'", | |||
There was a problem hiding this comment.
I think we can remove the player folder from lint and focus only on other files.
Player is here only for local testing
The goal is to make PapaParse friendlier for new developers.
|
As a general note I will prefer to not relax the style and keep the recomended features Thanks for your time on this. I will do a last review tomorrow before merging. |
The goal is to make PapaParse friendlier for new developers.
The eslint config was generated from
eslint --initwith some tweaking of the resulting rules to minimize errors.