Bug: import reads '\r' as a '\n' character

[javihernant]

Description
importing a file containing a carriage return ('\r') doesn't work as expected. import transforms the carriage return ('\r') into a newline ('\n'). This should be addressed because snapshot testing depends on this functionality to work. Take a look at denoland/std/issues/4226.

Steps to reproduce
I made a repro at https://github.com/javihernant/deno_import_test
deno run import.ts will import the snapshot.ts file and then print the object created by await import. There you'll see a string containing a '\n' instead of a '\r', that is what the snapshot.ts contains.

Deno version
Version: Deno 1.41.0

[dsherret]

Taking the code, renaming to a .js file, and modifying it to this:

export const snapshot = {};

snapshot[`test 1`] = `"
bye"`;
console.log(snapshot)

...while ensuring that a \r newline is still used in the template literal.

Deno, Node, and Chrome all output \n in this scenario. I'm not sure what the correct behaviour is, but if all three of those do the same thing then maybe this is v8 behaviour and I'm not sure if we should or can do anything here. Overall, it's probably not worth any additional effort because people shouldn't be using \r line endings these days.

[OzzieOrca]

If it's the case that \r shouldn't be used, should Deno's std/csv stringify function stop using the CRLF format as its newline output? https://github.com/denoland/deno_std/blob/main/csv/stringify.ts#L318 Or is that still needed for some systems/CSV programs.

I first noticed this issue when trying to snapshot the output of csv stringify. Maybe an alternative would be for assertSnapshot to strip \r from the actual input before trying to compare to the imported version. That makes assertSnapshot less strict and would succeed silently if someone was trying to care about a \r existing. But an error and no way to make the snapshot succeed (aside from manually stripping \r chars before) seems worse.

Not that this is an exhaustive list of programs but the one I was seeing commonly described as not able to handle \n was Microsoft Notepad. Seems like it can handle them now https://stackoverflow.com/a/50241693/665224.

[lucacasonato]

I think this behaviour is correct. std should generate snapshot files that do not end with \r in a string on a line.

[OzzieOrca]

Thanks for the clarification, we'll continue this discussion and what to do with \r characters in denoland/std#4226 then.