Skip to content

Conversation

nnaydenow
Copy link
Contributor

@nnaydenow nnaydenow commented Aug 19, 2025

Implement a caching mechanism for icon packages so they are regenerated only when there are changes in the icon packages or in the @ui5/webcomponents-tools package. This will reduce local build times when testing and also speed up the startup of the development server.

@ilhan007 ilhan007 requested a review from Copilot August 19, 2025 11:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a caching mechanism for icon packages to reduce build times by only regenerating icons when there are actual changes in the icon packages or the @ui5/webcomponents-tools package.

  • Adds a new hashing utility that tracks file changes using FNV-1a hash algorithm combined with modification times
  • Integrates hash checking into the icon build process to skip builds when no changes are detected
  • Updates build scripts to save and check hashes before performing expensive regeneration operations

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File Description
packages/tools/package.json Adds ignore dependency for .gitignore file parsing
packages/tools/lib/icons-hash/index.mjs New utility for computing and comparing file hashes to detect changes
packages/tools/icons-collection/nps.js Updates build scripts to check hashes before building and save hashes after building

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

try {
await fs.access(candidate);
gitignores.push(candidate);
} catch { }
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty catch block silently ignores all errors. Consider adding a comment explaining why errors are intentionally ignored, or handle specific expected errors like ENOENT.

Suggested change
} catch { }
} catch (err) {
// Ignore missing .gitignore files, but log other errors
if (err.code !== 'ENOENT') {
console.error(`Error accessing ${candidate}:`, err);
}
}

Copilot uses AI. Check for mistakes.

oldHashes = JSON.parse(raw);
} catch {
console.log(`No build hashes found for the ${getRepoName(repoPath)} package. Building it now.`);
process.exit(1);
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using process.exit() makes the code harder to test and reuse. Consider throwing an error or returning a status code that the caller can handle.

Suggested change
process.exit(1);
// Indicate missing hashes with a specific error
throw new Error("NO_HASHES_FOUND");

Copilot uses AI. Check for mistakes.

console.log(`No changes detected in the ${getRepoName(repoPath)} package.`);
} else {
console.log(`Changes detected in the ${getRepoName(repoPath)} package. Rebuilding it.`);
process.exit(2);
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using process.exit() makes the code harder to test and reuse. Consider throwing an error or returning a status code that the caller can handle.

Copilot uses AI. Check for mistakes.

const mode = process.argv[2];
if (!["save", "check"].includes(mode)) {
console.error("Usage: node hashes.js <save|check>");
process.exit(1);
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using process.exit() makes the code harder to test and reuse. Consider throwing an error or returning a status code that the caller can handle.

Suggested change
process.exit(1);
throw new Error("Usage: node hashes.js <save|check>");

Copilot uses AI. Check for mistakes.

oldHashes = JSON.parse(raw);
} catch {
console.log(`No build hashes found for the ${getRepoName(repoPath)} package. Building it now.`);
process.exit(1);
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty catch block silently ignores all errors. Consider adding a comment explaining why errors are intentionally ignored, or handle specific expected errors like file not found.

Suggested change
process.exit(1);
} catch (err) {
if (err && err.code === "ENOENT") {
console.log(`No build hashes found for the ${getRepoName(repoPath)} package. Building it now.`);
process.exit(1);
} else {
console.error(`Error reading build hashes for the ${getRepoName(repoPath)} package:`, err);
process.exit(2);
}

Copilot uses AI. Check for mistakes.

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.

1 participant