-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 latest fuels
version
#3350
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Peter Smith <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fuels
version
CodSpeed Performance ReportMerging #3350 will not alter performanceComparing Summary
|
Coverage Report:
Changed Files:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job 👏
const data: { version: string } | null = await Promise.race([ | ||
new Promise((resolve) => { | ||
setTimeout(() => resolve(null), 3000); | ||
}), | ||
fetch('https://registry.npmjs.org/fuels/latest').then((response) => response.json()), | ||
]); | ||
|
||
if (!data) { | ||
throw new Error('Failed to fetch latest fuels version.'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since an error is being thrown anyway, we can reject the promise instead of resolving it on line 11.
if (!cachedVersion) { | ||
return null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for the cache file to exist but cachedVersion
to be null?
const { mtimeMs: cacheTimestamp } = fs.statSync(FUELS_VERSION_CACHE_FILE); | ||
const hasCacheExpired = Date.now() - cacheTimestamp > FUELS_VERSION_CACHE_TTL; | ||
|
||
return hasCacheExpired ? null : cachedVersion; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth saving the timestamp along the cache so we don't need to call statSync
?
import fs from 'fs'; | ||
import path from 'path'; | ||
|
||
export const FUELS_VERSION_CACHE_FILE = path.join(__dirname, 'FUELS_VERSION'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should add this to the .gitignore
file
fuels
version #3177Summary
This PR adds a basic local cache for the latest
fuels
version fetched by thefuels
CLI.Checklist