gulp-terser-js assumes that OS EOL is the file's EOL

[shark-horse]

Encountered this with gulp-terser-js@5.0.0 on Windows, but not Linux:

[10:13:19] TypeError: Cannot read property 'replace' of undefined
    at printError (C:\myproject\node_modules\gulp-terser-js\bin\index.js:71:69)
    at DestroyableTransform._transform (C:\myproject\node_modules\gulp-terser-js\bin\index.js:42:19)
    at DestroyableTransform.Transform._read (C:\myproject\node_modules\gulp-terser-js\node_modules\readable-stream\lib\_stream_transform.js:177:10)
    at DestroyableTransform.Transform._write (C:\myproject\node_modules\gulp-terser-js\node_modules\readable-stream\lib\_stream_transform.js:164:83)
    at doWrite (C:\myproject\node_modules\gulp-terser-js\node_modules\readable-stream\lib\_stream_writable.js:405:139)
    at writeOrBuffer (C:\myproject\node_modules\gulp-terser-js\node_modules\readable-stream\lib\_stream_writable.js:394:5)
    at DestroyableTransform.Writable.write (C:\myproject\node_modules\gulp-terser-js\node_modules\readable-stream\lib\_stream_writable.js:303:11)
    at DestroyableTransform.ondata (C:\myproject\node_modules\readable-stream\lib\_stream_readable.js:619:20)
    at DestroyableTransform.emit (events.js:210:5)
    at DestroyableTransform.EventEmitter.emit (domain.js:499:23)
[10:13:19] 'build' errored after 8.42 s

That LoC is:

const pos = (more + 2) + fileContent.split(os.EOL)[error.line - 1].replace(/[^\t]/g, '').length * 3 + parseInt(col)

Replacing both instances of os.EOL in node_modules/gulp-terser-js/bin/index.js with '\n' fixes the problem in Windows so we get a nice error report. Replacing os.EOL with '\r' in Linux makes the same problem reproducible in Linux.

We have a lot of files with \n as the line ending. What I suspect is that terser is reporting line numbers correctly, but gulp-terser-js is not correlating that to the file because it is assuming that os.EOL is the same line ending that the file is using.

Just getting this information out there before I have time to confirm my theory, write a test, or submit an MR.

[A-312]

You have right, the current problem that we use the OS EOL and not the document EOL

let arr
try {
  arr = (more + 2) + fileContent.split(os.EOL)
} catch (e) {
  arr = (more + 2) + fileContent.split('\n')
}

Or :

if (text.match(/\n/g) && !text.match(/\r\n/g))

[shark-horse]

I would not try / catch for this, especially since an exception is not guaranteed: A file may have a mixture of \n and \r\n such that there are enough lines, so no TypeError, but I think the error message would show the wrong code.

Ideally, we would use the same newline characters that terser itself treats as newlines. I found: var NEWLINE_CHARS = makePredicate(characters("\n\r\u2028\u2029")); (source). In at least one spot \r\n is handled specially so that it is treated as just one newline, not two (source).

I'll work on a PR for this today if time allows.

[A-312]

@shark-horse Can you Q/A (try my change) #12 ?

I prefere to change the EOL because my function printError can be used for other package like gulp-less.

[shark-horse]

Yes, I will QA.

https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-262.pdf section 11.3 lists the same characters in terser's NEWLINE_CHARS as the line-terminating characters, with of course \r\n counting as one line terminator.

[shark-horse]

Please see my comment on the submission.