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: cache invalidation on .babelrc update #865

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

mellyeliu
Copy link
Member

What changed / motivation ?

Let's add cache invalidation on .babelrc config update

  • Iteratively search upwards through the file hierarchy until we find the closest .babelrc file
  • Hash file and add to cache

Testing

Added tests writing .babelrc update

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 17, 2025
@mellyeliu mellyeliu changed the base branch from main to cli-cache-config January 17, 2025 20:40
Copy link

github-actions bot commented Jan 17, 2025

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.

@stylexjs/[email protected] size:compare
./size-compare.js /tmp/tmp.ChA3UN45FI /tmp/tmp.JqW7IvfaUF

Results Base Patch Ratio
stylex/lib/stylex.js
· compressed 729 729 1.00
· minified 2,541 2,541 1.00
stylex/lib/StyleXSheet.js
· compressed 1,266 1,266 1.00
· minified 3,776 3,776 1.00
rollup-example/.build/bundle.js
· compressed 567,110 567,110 1.00
· minified 10,232,512 10,232,512 1.00
rollup-example/.build/stylex.css
· compressed 100,628 100,628 1.00
· minified 755,718 755,718 1.00

@mellyeliu mellyeliu requested a review from nmn January 17, 2025 21:41
Base automatically changed from cli-cache-config to feat-relative-cache-path January 18, 2025 00:10
Copy link
Contributor

@nmn nmn left a comment

Choose a reason for hiding this comment

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

One nitpick before I merge this.

Comment on lines +25 to +31
try {
await fs.access(babelrcPath);
console.log('Found babelrc:', babelrcPath);
return babelrcPath;
} catch {
currentDir = path.dirname(currentDir);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use fs.access. Use fs.exists instead. That returns a boolean and won't throw an error.

Copy link
Member Author

@mellyeliu mellyeliu Jan 21, 2025

Choose a reason for hiding this comment

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

https://nodejs.org/api/fs.html#fs_fs_exists_path_callback fs.exists is deprecated and may cause issues with modern node versions. if error specificity isn't a concern we can use fs.exists but it might require updates later, which could impact maintainability

@nmn
Copy link
Contributor

nmn commented Jan 18, 2025

Also needs to be rebased.

Base automatically changed from feat-relative-cache-path to main January 22, 2025 18:38
@mellyeliu mellyeliu requested a review from nmn January 23, 2025 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants