Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: only save files if they are unchanged #417

Merged
merged 1 commit into from Dec 10, 2021

Conversation

kentcdodds
Copy link
Contributor

Closes #320

This also solves a significant annoyance for anyone using postcss with an app that rebuilds on file change: remix-run/remix#714

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 30, 2021

Hey, code looks good; my only concern is performance. Has any testing been done on this?

@kentcdodds
Copy link
Contributor Author

kentcdodds commented Nov 30, 2021

Just did some testing here:

const fs = require("fs-extra");

async function oneFile() {
  console.time("one file");
  await fs.readFile("./files/01.css");
  console.timeEnd("one file");
}

async function hundredFiles() {
  console.time("100 files");
  for (let i = 0; i < 100; i++) {
    await fs.readFile(`./files/01 copy ${i}.css`);
  }
  console.timeEnd("100 files");
}

oneFile();
hundredFiles();

Those css files are each 111kb of this copy/pasted over and over:

body {
  color: red;
}

My output (on my fast machine) is:

one file: 0.557ms
100 files: 15.627ms

It's pretty darn fast. Feel free to watch for yourself (I'm live streaming right now): https://www.youtube.com/watch?v=KPKBUBchCVo

So I think this is fine. Though if you'd like to add an opt-out option then I could do that. But I kinda think that should be some added complexity that we should add only if necessary since this doesn't seem to be much of a problem.

@kentcdodds
Copy link
Contributor Author

Ping? 😅

@RyanZim
Copy link
Collaborator

RyanZim commented Dec 10, 2021

Hey, sorry for the slowness here; didn't want to merge until I had spare time to publish a new version.

@RyanZim RyanZim merged commit b06fa80 into postcss:master Dec 10, 2021
@kentcdodds kentcdodds deleted the kent/unchanged-support branch December 10, 2021 21:08
@kentcdodds
Copy link
Contributor Author

No worries. I know life is busy 😅 Thanks for merging and releasing this!

@chillitom
Copy link

could comparing files sizes first and only reading/comparing if different give a small perf improvement?

@kentcdodds
Copy link
Contributor Author

I expect it could, but it's it really worth the added complexity? For most cases it doesn't even take a full millisecond to just read and compare. It probably wouldn't make much of a difference...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skip file write if file content isn't changed
3 participants