Use write-file-atomic for writing fixed styles to filesystem

[ZhangYiJiang]

Which issue, if any, is this issue related to?

Fixes #2991

Is there anything in the PR that needs further explanation?

This makes writing back the linted results when --fix is used atomic - previously if the operation was cancelled or crashed while the styles were been written, the sources could be left in an inconsistent state.

The correct way to write file is to write it to a temp file first, then replace the original. As it turns out the nice folks at npm has written a package to do just that, and hey, if it's good enough for npm then it's probably good enough for Stylelint 😉

[ZhangYiJiang]

if it's good enough for npm then it's probably good enough for Stylelint 😉

Huh, looks I spoke too soon - this small change causes Stylelint to use a lot more memory than before, at least when running tests. On my laptop master uses ~450MB, while on this branch it ballooned to 1.4GB then crashed with an out of memory error 😱 I'm not entirely sure what is happening here, so I'll try to dig deeper.

[ZhangYiJiang]

So... erm, it seems that the culprit is graceful-fs, which is a transitive dependency of atomic-write-file - in fact this has actually came up before - jestjs/jest#2179 (comment) and #2168

I guess we could fork write-atomic-file and simply replace require('graceful-fs') with require('fs')...

[ZhangYiJiang]

Okay, yes, I can verify that a simple /s/graceful-fs/fs/g on write-file-atomic/index.js works. It does mean keeping a fork of the write-file-atomic package, which is, well, not ideal, but it works. The tests still leak a bit of memory, but at least they finish running without crashing.

Using the tmp module causes the same memory leak - see raszi/node-tmp#129 - this is what happens when I try to use that lib:

 PASS  system-tests/001/001.test.js (52 MB heap size)
 PASS  system-tests/004/004.test.js (62 MB heap size)
 PASS  lib/rules/at-rule-empty-line-before/__tests__/index.js (84 MB heap size)
 PASS  lib/rules/selector-no-qualifying-type/__tests__/index.js (106 MB heap size)
 PASS  lib/rules/value-keyword-case/__tests__/index.js (128 MB heap size)
 PASS  system-tests/005/005.test.js (140 MB heap size)
 PASS  lib/rules/selector-max-empty-lines/__tests__/index.js (162 MB heap size)
 PASS  lib/rules/selector-attribute-operator-space-before/__tests__/index.js (179 MB heap size)
 PASS  lib/rules/selector-attribute-operator-space-after/__tests__/index.js (195 MB heap size)
 PASS  lib/rules/declaration-empty-line-before/__tests__/index.js (210 MB heap size)
(node:15714) Warning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit
 PASS  lib/rules/color-named/__tests__/index.js (222 MB heap size)
 PASS  lib/rules/rule-empty-line-before/__tests__/index.js (236 MB heap size)
 PASS  lib/rules/selector-attribute-brackets-space-inside/__tests__/index.js (249 MB heap size)
 PASS  lib/rules/selector-pseudo-class-case/__tests__/index.js (263 MB heap size)
 PASS  lib/rules/custom-property-empty-line-before/__tests__/index.js (276 MB heap size)
 PASS  lib/rules/selector-class-pattern/__tests__/index.js (289 MB heap size)
 PASS  lib/rules/indentation/__tests__/rules.js (301 MB heap size)
 PASS  lib/rules/property-case/__tests__/index.js (315 MB heap size)
 PASS  lib/rules/function-url-quotes/__tests__/index.js (329 MB heap size)
 PASS  lib/__tests__/standalone-syntax.test.js (343 MB heap size)
 PASS  lib/rules/unit-case/__tests__/index.js (356 MB heap size)
 PASS  lib/rules/comment-empty-line-before/__tests__/index.js (369 MB heap size)
 PASS  lib/rules/selector-list-comma-space-before/__tests__/index.js (380 MB heap size)
 PASS  lib/rules/no-extra-semicolons/__tests__/index.js (393 MB heap size)
 PASS  lib/rules/selector-attribute-quotes/__tests__/index.js (404 MB heap size)
 PASS  lib/rules/shorthand-property-no-redundant-values/__tests__/index.js (416 MB heap size)
 PASS  lib/rules/selector-pseudo-element-case/__tests__/index.js (429 MB heap size)
 PASS  lib/rules/media-feature-name-case/__tests__/index.js (442 MB heap size)
 PASS  lib/__tests__/processors.test.js (452 MB heap size)
 PASS  lib/__tests__/ignore.test.js (464 MB heap size)
 PASS  lib/rules/selector-max-specificity/__tests__/index.js (477 MB heap size)
 PASS  lib/rules/block-closing-brace-space-after/__tests__/index.js (489 MB heap size)

[jeddy3]

Huh, looks I spoke too soon - this small change causes Stylelint to use a lot more memory than before, at least when running tests.

FYI, we had a memory leak with graceful-fs before. Forking write-atomic-file might be our only option.

[ZhangYiJiang]

@jeddy3 I believe this PR is ready for review

[jeddy3]

@ZhangYiJiang Sorry about the delay getting to this. LGTM!

[ntwb]

Linking to isaacs/node-graceful-fs#102 for if/when the issue is resolved

[ntwb]

LGTM

[ntwb]

Going to merge this in the hope it will fix the out of memory testing issues in #3069

Edit: It did not fix the memory issues in #3069

[ntwb]

Once again, thanks @ZhangYiJiang, sorry it took us so long to merge 😄