Repository: mholt/PapaParse
Pull request #1058 (2024-06-12 17:11:54)

Refactor header renaming logic to adress #1052, #1007
I refactored the header renaming logic to adress #1052, #1007
and updated test to match the improved renaming logic.

viralpickaxe: +1 on this we're seeing this in production!

Patch:
diff --git a/docs/faq.html b/docs/faq.html
index b8ecdb2..403461c 100644
--- a/docs/faq.html
+++ b/docs/faq.html
@@ -71,7 +71,7 @@
 					</p>
 
 					<h6 id="nodejs">Can I use Papa Parse server-side with Node.js?</h6>
-					<p>Yes, Paparse supports Node. See <a href="https://github.com/mholt/PapaParse/blob/master/README.md#papa-parse-for-node" target="_blank">our README</a>for further details.
+					<p>Yes, Paparse supports Node. See <a href="https://github.com/mholt/PapaParse/blob/master/README.md#papa-parse-for-node" target="_blank">our README</a> for further details.
 					</p>
 
 					<h6 id="dependencies">Does Papa Parse have any dependencies?</h6>
diff --git a/docs/resources/js/papaparse.js b/docs/resources/js/papaparse.js
index ac02f6d..9dd7fa2 100644
--- a/docs/resources/js/papaparse.js
+++ b/docs/resources/js/papaparse.js
@@ -491,6 +491,16 @@ License: MIT
 		this.parseChunk = function(chunk, isFakeChunk)
 		{
 			// First chunk pre-processing
+			const skipFirstNLines = parseInt(this._config.skipFirstNLines) || 0;
+			if (this.isFirstChunk && skipFirstNLines > 0) {
+				let _newline = this._config.newline;
+				if (!_newline) {
+					const quoteChar = this._config.quoteChar || '"';
+					_newline = this._handle.guessLineEndings(chunk, quoteChar);
+				}
+				const splitChunk = chunk.split(_newline);
+				chunk = [...splitChunk.slice(skipFirstNLines)].join(_newline);
+			}
 			if (this.isFirstChunk && isFunction(this._config.beforeFirstChunk))
 			{
 				var modifiedChunk = this._config.beforeFirstChunk(chunk);
@@ -503,7 +513,6 @@ License: MIT
 			// Rejoin the line we likely just split in two by chunking the file
 			var aggregate = this._partialLine + chunk;
 			this._partialLine = '';
-
 			var results = this._handle.parse(aggregate, this._baseIndex, !this._finished);
 
 			if (this._handle.paused() || this._handle.aborted()) {
@@ -1048,7 +1057,7 @@ License: MIT
 		{
 			var quoteChar = _config.quoteChar || '"';
 			if (!_config.newline)
-				_config.newline = guessLineEndings(input, quoteChar);
+				_config.newline = this.guessLineEndings(input, quoteChar);
 
 			_delimiterError = false;
 			if (!_config.delimiter)
@@ -1119,6 +1128,32 @@ License: MIT
 			_input = '';
 		};
 
+		this.guessLineEndings = function(input, quoteChar)
+		{
+			input = input.substr(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, '');
+
+			var r = input.split('\r');
+
+			var n = input.split('\n');
+
+			var nAppearsFirst = (n.length > 1 && n[0].length < r[0].length);
+
+			if (r.length === 1 || nAppearsFirst)
+				return '\n';
+
+			var numWithN = 0;
+			for (var i = 0; i < r.length; i++)
+			{
+				if (r[i][0] === '\n')
+					numWithN++;
+			}
+
+			return numWithN >= r.length / 2 ? '\r\n' : '\r';
+		};
+
 		function testEmptyLine(s) {
 			return _config.skipEmptyLines === 'greedy' ? s.join('').trim() === '' : s.length === 1 && s[0].length === 0;
 		}
@@ -1321,32 +1356,6 @@ License: MIT
 			};
 		}
 
-		function guessLineEndings(input, quoteChar)
-		{
-			input = input.substr(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, '');
-
-			var r = input.split('\r');
-
-			var n = input.split('\n');
-
-			var nAppearsFirst = (n.length > 1 && n[0].length < r[0].length);
-
-			if (r.length === 1 || nAppearsFirst)
-				return '\n';
-
-			var numWithN = 0;
-			for (var i = 0; i < r.length; i++)
-			{
-				if (r[i][0] === '\n')
-					numWithN++;
-			}
-
-			return numWithN >= r.length / 2 ? '\r\n' : '\r';
-		}
-
 		function addError(type, code, msg, row)
 		{
 			_results.errors.push({
diff --git a/papaparse.js b/papaparse.js
index 4bc3dc3..2616986 100755
--- a/papaparse.js
+++ b/papaparse.js
@@ -511,6 +511,16 @@ License: MIT
 		this.parseChunk = function(chunk, isFakeChunk)
 		{
 			// First chunk pre-processing
+			const skipFirstNLines = parseInt(this._config.skipFirstNLines) || 0;
+			if (this.isFirstChunk && skipFirstNLines > 0) {
+				let _newline = this._config.newline;
+				if (!_newline) {
+					const quoteChar = this._config.quoteChar || '"';
+					_newline = this._handle.guessLineEndings(chunk, quoteChar);
+				}
+				const splitChunk = chunk.split(_newline);
+				chunk = [...splitChunk.slice(skipFirstNLines)].join(_newline);
+			}
 			if (this.isFirstChunk && isFunction(this._config.beforeFirstChunk))
 			{
 				var modifiedChunk = this._config.beforeFirstChunk(chunk);
@@ -522,22 +532,6 @@ License: MIT
 
 			// Rejoin the line we likely just split in two by chunking the file
 			var aggregate = this._partialLine + chunk;
-			this._pendingSkip = parseInt(this._config.skipFirstNLines) || 0;
-			this._skipHeader = 0;
-			if (this._config.header) {
-				this._skipHeader++;
-			}
-			if (this._pendingSkip > 0) {
-				var splitChunk = aggregate.split('\n');
-				var currentChunkLength = splitChunk.length;
-				if (currentChunkLength <= this._pendingSkip) {
-					aggregate = this._partialLine;
-				}
-				else{
-					aggregate = this._partialLine + [...splitChunk.slice(0, this._skipHeader), ...splitChunk.slice(this._skipHeader + this._pendingSkip)].join('\n');
-				}
-				this._pendingSkip -= currentChunkLength;
-			}
 			this._partialLine = '';
 			var results = this._handle.parse(aggregate, this._baseIndex, !this._finished);
 
@@ -1093,7 +1087,7 @@ License: MIT
 		{
 			var quoteChar = _config.quoteChar || '"';
 			if (!_config.newline)
-				_config.newline = guessLineEndings(input, quoteChar);
+				_config.newline = this.guessLineEndings(input, quoteChar);
 
 			_delimiterError = false;
 			if (!_config.delimiter)
@@ -1167,6 +1161,32 @@ License: MIT
 			_input = '';
 		};
 
+		this.guessLineEndings = function(input, quoteChar)
+		{
+			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, '');
+
+			var r = input.split('\r');
+
+			var n = input.split('\n');
+
+			var nAppearsFirst = (n.length > 1 && n[0].length < r[0].length);
+
+			if (r.length === 1 || nAppearsFirst)
+				return '\n';
+
+			var numWithN = 0;
+			for (var i = 0; i < r.length; i++)
+			{
+				if (r[i][0] === '\n')
+					numWithN++;
+			}
+
+			return numWithN >= r.length / 2 ? '\r\n' : '\r';
+		};
+
 		function testEmptyLine(s) {
 			return _config.skipEmptyLines === 'greedy' ? s.join('').trim() === '' : s.length === 1 && s[0].length === 0;
 		}
@@ -1373,32 +1393,6 @@ License: MIT
 			};
 		}
 
-		function guessLineEndings(input, quoteChar)
-		{
-			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, '');
-
-			var r = input.split('\r');
-
-			var n = input.split('\n');
-
-			var nAppearsFirst = (n.length > 1 && n[0].length < r[0].length);
-
-			if (r.length === 1 || nAppearsFirst)
-				return '\n';
-
-			var numWithN = 0;
-			for (var i = 0; i < r.length; i++)
-			{
-				if (r[i][0] === '\n')
-					numWithN++;
-			}
-
-			return numWithN >= r.length / 2 ? '\r\n' : '\r';
-		}
-
 		function addError(type, code, msg, row)
 		{
 			var error = {
@@ -1486,61 +1480,14 @@ License: MIT
 			if (!input)
 				return returnable();
 
-			// Rename headers if there are duplicates
-			var firstLine;
-			if (config.header && !baseIndex)
-			{
-				firstLine = input.split(newline)[0];
-				var headers = firstLine.split(delim);
-				var separator = '_';
-				var headerMap = new Set();
-				var headerCount = {};
-				var duplicateHeaders = false;
-
-				// Using old-style 'for' loop to avoid prototype pollution that would be picked up with 'var j in headers'
-				for (var j = 0; j < headers.length; j++) {
-					var header = headers[j];
-					if (isFunction(config.transformHeader))
-						header = config.transformHeader(header, j);
-					var headerName = header;
-
-					var count = headerCount[header] || 0;
-					if (count > 0) {
-						duplicateHeaders = true;
-						headerName = header + separator + count;
-						// Initialise the variable if it hasn't been.
-						if (renamedHeaders === null) {
-							renamedHeaders = {};
-						}
-					}
-					headerCount[header] = count + 1;
-					// In case it already exists, we add more separators
-					while (headerMap.has(headerName)) {
-						headerName = headerName + separator + count;
-					}
-					headerMap.add(headerName);
-					if (count > 0) {
-						renamedHeaders[headerName] = header;
-					}
-				}
-				if (duplicateHeaders) {
-					var editedInput = input.split(newline);
-					editedInput[0] = Array.from(headerMap).join(delim);
-					input = editedInput.join(newline);
-				}
-			}
 			if (fastMode || (fastMode !== false && input.indexOf(quoteChar) === -1))
 			{
 				var rows = input.split(newline);
 				for (var i = 0; i < rows.length; i++)
 				{
 					row = rows[i];
-					// use firstline as row length may be changed due to duplicated headers
-					if (i === 0 && firstLine !== undefined) {
-						cursor += firstLine.length;
-					}else{
-						cursor += row.length;
-					}
+					cursor += row.length;
+
 					if (i !== rows.length - 1)
 						cursor += newline.length;
 					else if (ignoreLastRow)
@@ -1735,7 +1682,6 @@ License: MIT
 				break;
 			}
 
-
 			return finish();
 
 
@@ -1795,6 +1741,47 @@ License: MIT
 			/** Returns an object with the results, errors, and meta. */
 			function returnable(stopped)
 			{
+				if (config.header && !baseIndex && data.length)
+				{
+					const result = data[0];
+					const headerCount = {}; // To track the count of each base header
+					const usedHeaders = new Set(result); // To track used headers and avoid duplicates
+					let duplicateHeaders = false;
+
+					for (let i = 0; i < result.length; i++) {
+						let header = result[i];
+						if (isFunction(config.transformHeader))
+							header = config.transformHeader(header, i);
+
+						if (!headerCount[header]) {
+							headerCount[header] = 1;
+							result[i] = header;
+						} else {
+							let newHeader;
+							let suffixCount = headerCount[header];
+
+							// Find a unique new header
+							do {
+								newHeader = `${header}_${suffixCount}`;
+								suffixCount++;
+							} while (usedHeaders.has(newHeader));
+
+							usedHeaders.add(newHeader); // Mark this new Header as used
+							result[i] = newHeader;
+							headerCount[header]++;
+							duplicateHeaders = true;
+							if (renamedHeaders === null) {
+								renamedHeaders = {};
+							}
+							renamedHeaders[newHeader] = header;
+						}
+
+						usedHeaders.add(header); // Ensure the original header is marked as used
+					}
+					if (duplicateHeaders) {
+						console.warn('Duplicate headers found and renamed.');
+					}
+				}
 				return {
 					data: data,
 					errors: errors,


Test patch:
diff --git a/tests/test-cases.js b/tests/test-cases.js
index 68f367f..5eaa943 100644
--- a/tests/test-cases.js
+++ b/tests/test-cases.js
@@ -618,10 +618,10 @@ var CORE_PARSER_TESTS = [
 		input: 'c,c,c,c_1\n1,2,3,4',
 		config: { header: true },
 		expected: {
-			data: [['c', 'c_1', 'c_2', 'c_1_0'], ['1', '2', '3', '4']],
+			data: [['c', 'c_2', 'c_3', 'c_1'], ['1', '2', '3', '4']],
 			errors: [],
 			meta: {
-				renamedHeaders: {c_1: 'c', c_2: 'c'},
+				renamedHeaders: {c_2: 'c', c_3: 'c'},
 				cursor: 17
 			}
 		}
@@ -1585,11 +1585,11 @@ var PARSE_TESTS = [
 		}
 	},
 	{
-		description: "Skip First N number of lines , with header and 3 rows",
-		input: 'a,b,c,d\n1,2,3,4\n4,5,6,7',
+		description: "Skip First N number of lines , with header and 2 rows",
+		input: 'to-be-ignored\na,b,c,d\n1,2,3,4',
 		config: { header: true, skipFirstNLines: 1 },
 		expected: {
-			data: [{a: '4', b: '5', c: '6', d: '7'}],
+			data: [{a: '1', b: '2', c: '3', d: '4'}],
 			errors: []
 		}
 	},
@@ -1610,6 +1610,15 @@ var PARSE_TESTS = [
 			data: [['a','b','c','d'],['1','2','3','4'],['4','5','6','7']],
 			errors: []
 		}
+	},
+	{
+		description: "Skip first 2 lines , with custom newline character",
+		input: 'skip-this\rskip-this\r1,2,3,4',
+		config: { header: false, skipFirstNLines: 2, newline: '\r' },
+		expected: {
+			data: [['1','2','3','4']],
+			errors: []
+		}
 	}
 ];