-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
refactor: simplify lookupFile #12585
Changes from 1 commit
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 |
---|---|---|
|
@@ -17,7 +17,6 @@ import { | |
flattenId, | ||
getHash, | ||
isOptimizable, | ||
lookupFile, | ||
normalizeId, | ||
normalizePath, | ||
removeDir, | ||
|
@@ -1199,13 +1198,23 @@ const lockfileFormats = [ | |
{ name: 'pnpm-lock.yaml', checkPatches: false }, // Included in lockfile | ||
{ name: 'bun.lockb', checkPatches: true }, | ||
] | ||
const lockfileNames = lockfileFormats.map((l) => l.name) | ||
|
||
function findNearestLockfile(dir: string) { | ||
while (dir) { | ||
for (const fileName of lockfileNames) { | ||
const fullPath = path.join(dir, fileName) | ||
if (tryStatSync(fullPath)?.isFile()) return fullPath | ||
} | ||
const parentDir = path.dirname(dir) | ||
if (parentDir === dir) return | ||
|
||
dir = parentDir | ||
} | ||
} | ||
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. I think we can have a specialized version of the lookup here, this was the reason the second argument to 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. I think It'd still prefer the array version instead so we don't have two similar code 🤔 There shouldn't be a huge perf hit for the case where an array isn't needed too. 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. done, the function isn't used much now anyways |
||
|
||
export function getDepHash(config: ResolvedConfig, ssr: boolean): string { | ||
const lockfilePath = lookupFile( | ||
config.root, | ||
lockfileFormats.map((l) => l.name), | ||
{ pathOnly: true }, | ||
) | ||
const lockfilePath = findNearestLockfile(config.root) | ||
let content = lockfilePath ? fs.readFileSync(lockfilePath, 'utf-8') : '' | ||
if (lockfilePath) { | ||
const lockfileName = path.basename(lockfilePath) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,28 +387,16 @@ export function tryStatSync(file: string): fs.Stats | undefined { | |
// Ignore errors | ||
} | ||
} | ||
interface LookupFileOptions { | ||
pathOnly?: boolean | ||
rootDir?: string | ||
} | ||
|
||
export function lookupFile( | ||
dir: string, | ||
formats: string[], | ||
options?: LookupFileOptions, | ||
): string | undefined { | ||
for (const format of formats) { | ||
const fullPath = path.join(dir, format) | ||
if (tryStatSync(fullPath)?.isFile()) { | ||
return options?.pathOnly ? fullPath : fs.readFileSync(fullPath, 'utf-8') | ||
} | ||
} | ||
const parentDir = path.dirname(dir) | ||
if ( | ||
parentDir !== dir && | ||
(!options?.rootDir || parentDir.startsWith(options?.rootDir)) | ||
) { | ||
return lookupFile(parentDir, formats, options) | ||
export function lookupFile(dir: string, fileName: string): string | undefined { | ||
while (dir) { | ||
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. Removed the recursion, this is the same scheme used by @bluwy in findNearestPackageData. |
||
const fullPath = path.join(dir, fileName) | ||
if (tryStatSync(fullPath)?.isFile()) return fullPath | ||
|
||
const parentDir = path.dirname(dir) | ||
if (parentDir === dir) return | ||
|
||
dir = parentDir | ||
} | ||
} | ||
|
||
|
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.
We were passing
rootDir: envDir
here, so only level would be checked bylookupFile
but the call makes it seems env dirs are being looked up in the fs. Replaced it with a singletryStatSync