verifyTypeScriptSetup.js writes mixed line endings into tsconfig.json

[mikebeaton]

In verifyTypeScriptSetup.js:

    fs.writeFileSync(fileName, JSON.stringify(object, null, 2) + os.EOL);

the code uses a combination of JSON.stringify and os.EOL - however JSON.stringify is specified to (and does) always use 0x000A as its linefeed, even on Windows.

The net result is that after building a create-react-app project on Windows, then tsconfig.json contains LFs all down the file until the last line, which is CRLF, as shown here:

• This is not a user misconfiguration, it is the out-of-the-box behaviour
• It occurs whenever verifyTypeScriptSetup.js rewrites this file on Windows
• tsconfig.json ends up with linebreaks as shown above regardless of whether it was initially pure LF or initially pure CRLF

It is possible to achieve (what seems to be fairly evidently) the intent of the code by replacing all LF characters in the JSON.stringify output with os.EOL as follows:

    fs.writeFileSync(fileName, JSON.stringify(object, null, 2).replace(/\n/g, os.EOL) + os.EOL);

NB Even this is NOT necessarily what the user actually wants (for a Windows user, it depends on whether they are working with Windows linefeeds, or perhaps in Unix linefeeds because it is a cross-platform project).

I'd be happy to do a PR, but suspect there may well be other places where this approach is used, and also note that there are two other uses of os.EOL in the same file (which again may or may not be what the user really wants for the reason just mentioned).

[mikebeaton]

If anybody from the create-react-app team is looking at this, I'd be happy to put in the fixed line above as a PR - but note the following which I think should be resolved/discussed first:

• There seem to be potentially related issues which this alone might not fix (as mentioned above)
• The simple fix (as suggested above) to the code line which has the problem will achieve what (it seems clear) the code was intended to do (but doesn't!), but still won't always do what the user actually wants
  • Any suggestions as to any way to make the line ending used default to the correct for the OS - which I can do - but also be configurable if the user needs it to be would be good
  • Actually, off the top of my head, I'd suggest that the tsconfig.json file itself could have a new config value saying which line endings it should use if/when re-written, with values 'windows', 'unix' and 'native' available, and defaulting to 'native' (c.f. this eslint project issue - which somewhat naturally suggests that the new tsconfig.json setting could natually be called 'linebreak-style' - but perhaps would be better as 'self-linebreak-style', to make it clear what it applies to)
  • Again, I'd be happy to do the above as a 'more complete' PR (in the sense that it would actually, rather than only partially, solve the problems which the very fact of a re-write can potentially cause), and it would be fairly simple code, in the style of the existing file (if the approach just suggested seems reasonable to the team?), but still see the following point
• I am not aware of what tests it might be appropriate to add for such a fix, and a suggestion as to where and what would help

[ianschmitz]

I think .replace(/\r\n|\r|\n/g, os.EOL) should be sufficient for most everyone.

[mikebeaton]

@ianschmitz I believe that .replace(/\n/g, os.EOL) is sufficient, since JSON.stringify always and only produces \n (it would be in breach of its contract otherwise).

A PR for that and then (if I or someone else cares enough) a separate Issue and discussion as to whether or not it's worth doing anything at all about the small class of Windows users who have Git set to check out Unix linefeeds, would make sense.

[mikebeaton]

Dear Ian, I've put up a PR as requested, I'd obviously be happy to change it to use the more defensive coding you've suggested, if you'd all prefer.

[For some reason, CI builds on the project are failing for some reason nothing to do with the code pushed up - in the eject tests, the (docker?) image is causing the process to fail with a missing package error.] - EDIT: Now fixed.

[mikebeaton]

Just wanted to leave a note that the fix as applied works for users using native linefeeds (i.e. most people!), so many thanks! But still won't work for users using Unix linefeeds on Windows (or any other weird combination, but this is probably much the most likely one, and is even the recommended setting by eslint!).