[astro-purgecss] Fix race-condition in renaming css files (#662)

* [astro-purgecss] Fix race-condition in calls to replaceValueInFile - fix #660

Ensure that all CSS file references needing to be updated are done before saving HTML files.

The process has been refactored in order to:

* read HTML files only once
* replace all CSS file references which need to be changed (for loop)
* write updated HTML file to disk.

As a side-effect, build performance should also be improved (previous version was reading and writing file each time a CSS file reference had to be updated).

* [docs] correct changeset command

* docs(changeset): Fix race-condition in calls to replaceValueInFile

* fix tests

.changeset/fifty-eggs-compare.md

@@ -0,0 +1,5 @@
+---
+'astro-purgecss': patch
+---
+
+Fix race-condition in calls to replaceValueInFile

CONTRIBUTING.md

@@ -25,7 +25,7 @@ To include a changeset in your PR:
 1. Create a changeset using the following command within your project directory:
 
 ```bash
-pnpm changesets
+pnpm changeset
 ```
 
 2. Follow the prompts to describe the changes introduced in your PR. Ensure the changeset adheres to our contribution guidelines.

packages/astro-purgecss/src/index.ts

@@ -4,7 +4,9 @@ import { PurgeCSS, type UserDefinedOptions } from 'purgecss';
 import {
   cleanPath,
   headline,
+  readFileContent,
   replaceValueInFile,
+  saveUpdatedFile,
   success,
   writeCssFile
 } from './utils';
@@ -61,19 +63,22 @@ function Plugin(options: PurgeCSSOptions = {}): AstroIntegration {
 
         await Promise.all(
           pages.map(async (page) => {
-            await Promise.all(
-              processed.map(async ([oldFile, newFile]) => {
-                // Replace only if name of the old file
-                // is different from name of the new file (hash changes)
-                if (oldFile !== newFile) {
-                  await replaceValueInFile(
-                    page,
-                    oldFile.replace(outDir, ''),
-                    newFile.replace(outDir, '')
-                  );
-                }
-              })
-            );
+
+            let fileContent = await readFileContent(page);
+
+            for (const [oldFile, newFile] of processed) {
+              // Replace only if name of the old file
+              // is different from name of the new file (hash changes)
+              if (oldFile !== newFile) {
+                fileContent = replaceValueInFile(
+                  page,
+                  fileContent,
+                  oldFile.replace(outDir, ''),
+                  newFile.replace(outDir, '')
+                );
+              }
+			}
+			await saveUpdatedFile(page, fileContent);
             success(page.replace(outDir, ''));
           })
         );

packages/astro-purgecss/src/utils.test.ts

@@ -1,7 +1,7 @@
 import { readFile, unlink, writeFile } from 'node:fs/promises';
 import { afterEach, describe, expect, it, vi } from 'vitest';
 
-import { cleanPath, replaceValueInFile, writeCssFile } from './utils';
+import { cleanPath, readFileContent, replaceValueInFile, saveUpdatedFile, writeCssFile } from './utils';
 
 vi.mock('node:fs/promises', () => ({
   readFile: vi.fn(() => Promise.resolve()),
@@ -123,13 +123,12 @@ describe('replaceValueInFile', () => {
     const originalContent = 'This is the oldValue example.';
     const expectedContent = 'This is the newValue example.';
 
-    // Mock the file content
-    vi.mocked(readFile).mockResolvedValue(originalContent);
+    const replaceValueInFileSpy = vi.fn(replaceValueInFile)
 
-    await replaceValueInFile(filePath, searchValue, replaceValue);
+    await replaceValueInFileSpy(filePath, originalContent, searchValue, replaceValue);
 
     // Expect the file to be written with the new content
-    expect(writeFile).toHaveBeenCalledWith(filePath, expectedContent, 'utf8');
+    expect(replaceValueInFileSpy).toHaveReturnedWith(expectedContent);
   });
 
   it('should not modify the file if the search value is not found', async () => {
@@ -141,27 +140,43 @@ describe('replaceValueInFile', () => {
     // Mock the file content
     vi.mocked(readFile).mockResolvedValue(originalContent);
 
-    await replaceValueInFile(filePath, searchValue, replaceValue);
+    await replaceValueInFile(filePath, originalContent, searchValue, replaceValue);
 
     // Expect the file not to be written
     expect(writeFile).not.toHaveBeenCalled();
   });
 
-  it("should log an error if there's an error reading or writing the file", async () => {
+  it("should log an error if there's an error reading a file", async () => {
     const filePath = '/path/to/file.txt';
-    const searchValue = 'oldValue';
-    const replaceValue = 'newValue';
     const error = new Error('File operation failed');
 
     // Mock the file content to throw an error
     vi.mocked(readFile).mockRejectedValue(error);
     vi.spyOn(console, 'error');
 
-    await replaceValueInFile(filePath, searchValue, replaceValue);
+    await readFileContent(filePath);
 
     // Expect the error to be logged
     expect(console.error).toHaveBeenCalledWith(
-      `Error processing file ${filePath}: ${error}`
+      `Error reading file ${filePath}: ${error}`
     );
   });
+
+  it("should log an error if there's an error saving a file", async () => {
+    const filePath = '/path/to/file.txt';
+    const newContent = 'file content';
+    const error = new Error('File operation failed');
+
+    // Mock the file content to throw an error
+    vi.mocked(writeFile).mockRejectedValue(error);
+    vi.spyOn(console, 'error');
+
+    await saveUpdatedFile(filePath, newContent);
+
+    // Expect the error to be logged
+    expect(console.error).toHaveBeenCalledWith(
+      `Error saving updated file ${filePath}: ${error}`
+    );
+  });
+
 });

packages/astro-purgecss/src/utils.ts

@@ -3,22 +3,49 @@ import { createHash } from 'node:crypto';
 import { readFile, unlink, writeFile } from 'node:fs/promises';
 import { fileURLToPath } from 'node:url';
 
-export async function replaceValueInFile(
+export async function readFileContent(
+  filePath: string
+): Promise<string> {
+  try {
+    const content = await readFile(filePath, 'utf8');
+    return content;
+  } catch (err) {
+    console.error(`Error reading file ${filePath}: ${err}`);
+    return '';
+  }
+}
+
+export function replaceValueInFile(
   filePath: string,
+  content: string,
   searchValue: string,
   replaceValue: string
-) {
+): string {
   try {
-    const content = await readFile(filePath, 'utf8');
     if (content.includes(searchValue)) {
       const newContent = content.replace(
         new RegExp(searchValue, 'g'),
         replaceValue
       );
-      await writeFile(filePath, newContent, 'utf8');
+      return newContent;
+    }
+    else {
+      return content;
     }
   } catch (err) {
     console.error(`Error processing file ${filePath}: ${err}`);
+    return '';
+  }
+}
+
+export async function saveUpdatedFile(
+  filePath: string,
+  newContent: string
+) {
+  try {
+    await writeFile(filePath, newContent, 'utf8');
+  } catch (err) {
+    console.error(`Error saving updated file ${filePath}: ${err}`);
   }
 }