-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(cli): Add check-client command to verify bundle freshness #7517
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
base: main
Are you sure you want to change the base?
Changes from all commits
79d7864
b4c6899
179de2f
1075962
df4758b
ac2b4ef
970334a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@builder.io/qwik-city': patch | ||
'@builder.io/qwik': patch | ||
--- | ||
|
||
feat(cli): Add check-client command to verify bundle freshness |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
import type { AppCommand } from '../utils/app-command'; | ||
import { intro, log, outro } from '@clack/prompts'; | ||
import { bgBlue, bgMagenta, bold, cyan, red } from 'kleur/colors'; | ||
import { runInPkg } from '../utils/install-deps'; | ||
import { getPackageManager, panic } from '../utils/utils'; | ||
import fs from 'fs/promises'; | ||
import type { Stats } from 'fs'; | ||
import path from 'path'; | ||
|
||
const getDiskPath = (dist: string) => path.resolve(dist); | ||
const getSrcPath = (src: string) => path.resolve(src); | ||
const getManifestPath = (dist: string) => path.resolve(dist, 'q-manifest.json'); | ||
|
||
export async function runQwikClientCommand(app: AppCommand) { | ||
try { | ||
const src = app.args[1]; | ||
const dist = app.args[2]; | ||
await checkClientCommand(app, src, dist); | ||
} catch (e) { | ||
console.error(`❌ ${red(String(e))}\n`); | ||
process.exit(1); | ||
} | ||
} | ||
|
||
/** | ||
* Handles the core logic for the 'check-client' command. Exports this function so other modules can | ||
* import and call it. | ||
* | ||
* @param {AppCommand} app - Application command context (assuming structure). | ||
*/ | ||
async function checkClientCommand(app: AppCommand, src: string, dist: string): Promise<void> { | ||
if (!(await clientDirExists(dist))) { | ||
intro(`🚀 ${bgBlue(bold(' Qwik Client Check '))}`); | ||
log.step(`Checking Disk directory: ${cyan(dist)}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this |
||
await goBuild(app); | ||
} else { | ||
log.step(`Checking q-manifest.json file: ${cyan(dist)}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this |
||
const manifest = await getManifestTs(getManifestPath(dist)); | ||
if (manifest === null) { | ||
await goBuild(app); | ||
} else { | ||
log.step(`Compare source modification time with q-manifest.json modification time`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this |
||
if (await isNewerThan(getSrcPath(src), manifest)) { | ||
await goBuild(app); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Builds the application using the appropriate package manager. | ||
* | ||
* @param {AppCommand} app - The application command object containing app details.e path to the | ||
* manifest file (though it's not used in the current function). | ||
* @throws {Error} Throws an error if the build process encounters any issues. | ||
*/ | ||
|
||
async function goBuild(app: AppCommand) { | ||
const pkgManager = getPackageManager(); | ||
log.step('Building client (manifest missing or outdated)...'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can make this |
||
try { | ||
const { install } = await runInPkg(pkgManager, ['run', 'build.client'], app.rootDir); | ||
if (!(await install)) { | ||
throw new Error('Client build command reported failure.'); | ||
} | ||
} catch (buildError: any) { | ||
log.error(`Build error: ${buildError.message}`); | ||
throw new Error('Client build process encountered an error.'); | ||
} | ||
Comment on lines
+66
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for the catch, runInPkg already does that |
||
outro(`✅ ${bgMagenta(bold(' Check complete '))}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this to say |
||
} | ||
|
||
/** | ||
* Retrieves the last modified timestamp of the manifest file. | ||
* | ||
* @param {string} manifestPath - The path to the manifest file. | ||
* @returns {Promise<number | null>} Returns the last modified timestamp (in milliseconds) of the | ||
* manifest file, or null if an error occurs. | ||
*/ | ||
async function getManifestTs(manifestPath: string) { | ||
try { | ||
// Get stats for the manifest file | ||
const stats: Stats = await fs.stat(manifestPath); | ||
return stats.mtimeMs; | ||
} catch (err: any) { | ||
// Handle errors accessing the manifest file | ||
if (err.code === 'ENOENT') { | ||
log.warn(`q-manifest.json file not found`); | ||
} else { | ||
panic(`Error accessing manifest file ${manifestPath}: ${err.message}`); | ||
} | ||
return null; | ||
} | ||
} | ||
|
||
/** | ||
* Checks if the specified disk directory exists and is accessible. | ||
* | ||
* @returns {Promise<boolean>} Returns true if the directory exists and can be accessed, returns | ||
* false if it doesn't exist or an error occurs. | ||
*/ | ||
export async function clientDirExists(dist: string): Promise<boolean> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since you're using it for both src and dist, and it doesn't have anything to do with the client, you should rename this to just |
||
try { | ||
await fs.access(getDiskPath(dist)); | ||
return true; // Directory exists | ||
} catch (err: any) { | ||
panic(`Error accessing disk directory ${dist}: ${err.message}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the err.code is ENOENT then there's no need for panic |
||
return false; // Directory doesn't exist or there was an error | ||
} | ||
} | ||
|
||
/** | ||
* Recursively finds the latest modification time (mtime) of any file in the given directory. | ||
* | ||
* @param {string} srcPath - The directory path to search. | ||
* @returns {Promise<number>} Returns the latest mtime (Unix timestamp in milliseconds), or 0 if the | ||
* directory doesn't exist or is empty. | ||
*/ | ||
export async function isNewerThan(srcPath: string, timestamp: number): Promise<boolean> { | ||
let returnValue = false; | ||
async function traverse(dir: string): Promise<void> { | ||
if (returnValue) { | ||
return; | ||
} | ||
let items: Array<import('fs').Dirent>; | ||
try { | ||
items = await fs.readdir(dir, { withFileTypes: true }); | ||
} catch (err: any) { | ||
if (err.code !== 'ENOENT') { | ||
console.warn(`Cannot read directory ${dir}: ${err.message}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be log.warn |
||
} | ||
return; | ||
} | ||
|
||
for (const item of items) { | ||
const fullPath = path.join(dir, item.name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. put the |
||
try { | ||
if (item.isDirectory()) { | ||
await traverse(fullPath); | ||
} else if (item.isFile()) { | ||
const stats = await fs.stat(fullPath); | ||
if (stats.mtimeMs > timestamp) { | ||
returnValue = true; | ||
return; | ||
} | ||
} | ||
} catch (err: any) { | ||
console.warn(`Cannot access ${fullPath}: ${err.message}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
} | ||
} | ||
} | ||
|
||
await traverse(srcPath); | ||
return returnValue; | ||
} |
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.
remove this