Repository: mholt/PapaParse
Pull request #724 (2019-10-21 16:04:42)

Address deepEqual using compare by JSON strings.
1) JSON does not guarantee the ordering that fields are encoded in the string, making it unreliable for generic hashing.
2) When ordering is maintained, the strings match the ordering the fields were added to the object, making the unit test too tighly coupled to the order they are added in the implementation.
3) A field whose value is undefined versus not being present presents a difference that deepEqual should assert.  If it does not matter then possibly a different assert should be used.


Patch:
diff --git a/papaparse.js b/papaparse.js
index 22a6a7e..d04eaae 100755
--- a/papaparse.js
+++ b/papaparse.js
@@ -699,7 +699,7 @@ License: MIT
 			if (contentRange === null) { // no content range, then finish!
 				return -1;
 			}
-			return parseInt(contentRange.substr(contentRange.lastIndexOf('/') + 1));
+			return parseInt(contentRange.substring(contentRange.lastIndexOf('/') + 1));
 		}
 	}
 	NetworkStreamer.prototype = Object.create(ChunkStreamer.prototype);
@@ -788,8 +788,14 @@ License: MIT
 		{
 			if (this._finished) return;
 			var size = this._config.chunkSize;
-			var chunk = size ? remaining.substr(0, size) : remaining;
-			remaining = size ? remaining.substr(size) : '';
+			var chunk;
+			if(size) {
+				chunk = remaining.substring(0, size);
+				remaining = remaining.substring(size);
+			} else {
+				chunk = remaining;
+				remaining = '';
+			}
 			this._finished = !remaining;
 			return this.parseChunk(chunk);
 		};
@@ -1094,7 +1100,7 @@ License: MIT
 		{
 			_paused = true;
 			_parser.abort();
-			_input = _input.substr(_parser.getCharIndex());
+			_input = _input.substring(_parser.getCharIndex());
 		};
 
 		this.resume = function()
@@ -1332,7 +1338,7 @@ License: MIT
 
 		function guessLineEndings(input, quoteChar)
 		{
-			input = input.substr(0, 1024 * 1024);	// max length 1 MB
+			input = input.substring(0, 1024 * 1024);	// max length 1 MB
 			// Replace all the text inside quotes
 			var re = new RegExp(escapeRegExp(quoteChar) + '([^]*?)' + escapeRegExp(quoteChar), 'gm');
 			input = input.replace(re, '');
@@ -1358,12 +1364,15 @@ License: MIT
 
 		function addError(type, code, msg, row)
 		{
-			_results.errors.push({
+			var error = {
 				type: type,
 				code: code,
-				message: msg,
-				row: row
-			});
+				message: msg
+			};
+			if(row !== undefined) {
+				error.row = row;
+			}
+			_results.errors.push(error);
 		}
 	}
 
@@ -1450,7 +1459,7 @@ License: MIT
 						cursor += newline.length;
 					else if (ignoreLastRow)
 						return returnable();
-					if (comments && row.substr(0, commentsLen) === comments)
+					if (comments && row.substring(0, commentsLen) === comments)
 						continue;
 					if (stepIsFunction)
 					{
@@ -1553,7 +1562,7 @@ License: MIT
 						var spacesBetweenQuoteAndNewLine = extraSpaces(nextNewline);
 
 						// Closing quote followed by newline or 'unnecessary spaces + newLine'
-						if (input.substr(quoteSearch + 1 + spacesBetweenQuoteAndNewLine, newlineLen) === newline)
+						if (input.substring(quoteSearch + 1 + spacesBetweenQuoteAndNewLine, quoteSearch + 1 + spacesBetweenQuoteAndNewLine + newlineLen) === newline)
 						{
 							row.push(input.substring(cursor, quoteSearch).replace(quoteCharRegex, quoteChar));
 							saveRow(quoteSearch + 1 + spacesBetweenQuoteAndNewLine + newlineLen);
@@ -1592,7 +1601,7 @@ License: MIT
 				}
 
 				// Comment found at start of new line
-				if (comments && row.length === 0 && input.substr(cursor, commentsLen) === comments)
+				if (comments && row.length === 0 && input.substring(cursor, cursor + commentsLen) === comments)
 				{
 					if (nextNewline === -1)	// Comment ends at EOF
 						return returnable();
@@ -1684,7 +1693,7 @@ License: MIT
 				if (ignoreLastRow)
 					return returnable();
 				if (typeof value === 'undefined')
-					value = input.substr(cursor);
+					value = input.substring(cursor);
 				row.push(value);
 				cursor = inputLen;	// important in case parsing is paused
 				pushRow(row);


Test patch:
diff --git a/tests/test-cases.js b/tests/test-cases.js
index 58a85f0..608b297 100644
--- a/tests/test-cases.js
+++ b/tests/test-cases.js
@@ -592,7 +592,7 @@ describe('Core Parser Tests', function() {
 	function generateTest(test) {
 		(test.disabled ? it.skip : it)(test.description, function() {
 			var actual = new Papa.Parser(test.config).parse(test.input);
-			assert.deepEqual(JSON.stringify(actual.errors), JSON.stringify(test.expected.errors));
+			assert.deepEqual(actual.errors, test.expected.errors);
 			assert.deepEqual(actual.data, test.expected.data);
 		});
 	}
@@ -1475,7 +1475,7 @@ describe('Parse Tests', function() {
 			if (test.expected.meta) {
 				assert.deepEqual(actual.meta, test.expected.meta);
 			}
-			assert.deepEqual(JSON.stringify(actual.errors), JSON.stringify(test.expected.errors));
+			assert.deepEqual(actual.errors, test.expected.errors);
 			assert.deepEqual(actual.data, test.expected.data);
 		});
 	}
@@ -1556,7 +1556,7 @@ describe('Parse Async Tests', function() {
 			var config = test.config;
 
 			config.complete = function(actual) {
-				assert.deepEqual(JSON.stringify(actual.errors), JSON.stringify(test.expected.errors));
+				assert.deepEqual(actual.errors, test.expected.errors);
 				assert.deepEqual(actual.data, test.expected.data);
 				done();
 			};
@@ -2384,7 +2384,7 @@ describe('Custom Tests', function() {
 				this.timeout(test.timeout);
 			}
 			test.run(function(actual) {
-				assert.deepEqual(JSON.stringify(actual), JSON.stringify(test.expected));
+				assert.deepEqual(actual, test.expected);
 				done();
 			});
 		});