Do not rewrite files when content did not change

[rogeliog]

First of all thanks for all the great work on this package,👏 it has been super useful!

I know that there is no open issue for this, so feel free to close this PR if this does not go with the direction of the project.

TL;DR

Don't write a new version of the .css.flow file if the contents did not change.

The problem

Every time that a file is changed our git repo gets dirty, making it hard to do

❯ git status
On branch my-banrch
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   src/components/Component1/styles.css.flow
	modified:   src/components/Component2/styles.css.flow

❯ git diff
diff --git a/src/components/Component1/styles.css.flow b/src/components/Component1/styles.css.flow
index 9c8e505..e69de29 100644
--- a/src/components/Component1/styles.css.flow
+++ b/src/components/Component1/styles.css.flow
@@ -1,9 +0,0 @@
-// @flow
-/* This file is automatically generated by css-modules-flow-types */
-declare module.exports: {|
-  +'class1': string;
-  +'class2': string;
-  +'class3': string;
-|};

diff --git a/src/components/Component2/styles.css.flow b/src/components/Component2/styles.css.flow
index 9c8e505..e69de29 100644
--- a/src/components/Component2/styles.css.flow
+++ b/src/components/Component2/styles.css.flow
@@ -1,9 +0,0 @@
-// @flow
-/* This file is automatically generated by css-modules-flow-types */
-declare module.exports: {|
-  +'class1': string;
-  +'class2': string;
-  +'class3': string;
-|};

[skovhus]

Thanks for contributing! Makes sense not to write file more than nessesary, but I don't see why your git repo gets dirty. It is simply touching the file.

[rogeliog]

:man_facepalming: sorry, I forgot to add that to the description, my bad...

I still don't understand why it is taking so long to write to do the write, but here is what I have found.

My primitive benchmarking

I added a really primitive benchmarking, in my local setup

const start = +new Date();
fs.writeFile(outputPath, printFlowDefinition(tokens), {}, function() {
   console.log(+new Date() - start);
});

And this gives me values that range from 1ms to 2000ms... I wonder if it because there are too many writeFile happening at the same time.

Info about the project

Node version: 8.6.0 webpack version: 1.13.0 Number of css files: 196

[rogeliog]

Further investigation

It seems that typings-for-css-modules-loader is also just writing to file it the contents changed https://github.com/Jimdo/typings-for-css-modules-loader/blob/master/src/persist.js

[skovhus]

sorry, I forgot to add that to the description, my bad...

Still missing it seems. : ) I would like to understand why writing/touching a file is a problem on your system. It should not cause any unstated files in git.

Are you checking in the .css.flow files?

Performance slow down is the reason why I'm not reading the file. IO is expensive here. Especially inside a webpack loader.

[skovhus]

In the diffs shown here it look like the file content were removed... or am I missing something?

[rogeliog]

Yes I'm checking in the .css.flow files, I'm working in a minimal repro case... I'll post it soon

[rogeliog]

Here is a minimal repro case for this issue ttps://github.com/rogeliog/fs-write-weird-issue

I don't know exactly why it happens, but think it has to do with how fs.writeFile works... in that repo I have a script that continuously writes to 100 files to disk.

I wonder if we should use fs.writeFileSync instead(which is what typings-for-css-modules-loader is using)

See https://github.com/rogeliog/fs-write-weird-issue for more details about this gif

[rogeliog]

Any thoughts?

[skovhus]

@rogeliog sorry I've been a bit busy. It seems like this is an issue on some machines... Wondering a bit why I haven't seen the issue before.

I wonder if we should use fs.writeFileSync instead(which is what typings-for-css-modules-loader is using)

Not sure if that will slow down webpack a lot. But please give it a try if you have time.

Can you see how much reading and writing the file affect webpack performance? Like you touch a .css file and see how long a rebuild takes.

[skovhus]

Thanks for the reproducible repo!

[silvenon]

To me this seems like a good idea in general (performance-wise), regardless of the bug.

[silvenon]

How about using a local cache to store file content based on this.resourcePath? That way you can check contents without IO.

[skovhus]

How about using a local cache to store file content based on this.resourcePath? That way you can check contents without IO.

Good idea. 👍

[skovhus]

@silvenon @rogeliog not sure if you are still using this project?

I’m not actively using it anymore, but I’m open for a revamp of this PR.

[silvenon]

I'm not using Flow nor CSS Modules anymore, so not invested 🤷