Skip to content

Commit

Permalink
Optimize synchronous getFileCommitDate code
Browse files Browse the repository at this point in the history
  • Loading branch information
slorber committed Feb 24, 2024
1 parent 0279c32 commit 0da5877
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ describe('blog plugin', () => {
const noDateSource = path.posix.join('@site', PluginPath, 'no date.md');
const noDateSourceFile = path.posix.join(siteDir, PluginPath, 'no date.md');
// We know the file exists and we know we have git
const result = getFileCommitDate(noDateSourceFile, {age: 'oldest'});
const result = await getFileCommitDate(noDateSourceFile, {age: 'oldest'});
const noDateSourceTime = result.date;

expect({
Expand Down
3 changes: 2 additions & 1 deletion packages/docusaurus-plugin-content-blog/src/blogUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,11 @@ async function processBlogSourceFile(
}

try {
const result = getFileCommitDate(blogSourceAbsolute, {
const result = await getFileCommitDate(blogSourceAbsolute, {
age: 'oldest',
includeAuthor: false,
});

return result.date;
} catch (err) {
logger.warn(err);
Expand Down
3 changes: 2 additions & 1 deletion packages/docusaurus-plugin-content-docs/src/lastUpdate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ export async function getFileLastUpdate(
// Wrap in try/catch in case the shell commands fail
// (e.g. project doesn't use Git, etc).
try {
const result = getFileCommitDate(filePath, {
const result = await getFileCommitDate(filePath, {
age: 'newest',
includeAuthor: true,
});

return {timestamp: result.timestamp, author: result.author};
} catch (err) {
if (err instanceof GitNotFoundError) {
Expand Down
40 changes: 22 additions & 18 deletions packages/docusaurus-utils/src/__tests__/gitUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,88 +47,92 @@ function initializeTempRepo() {
describe('getFileCommitDate', () => {
const repoDir = initializeTempRepo();
it('returns earliest commit date', async () => {
expect(getFileCommitDate(path.join(repoDir, 'test.txt'), {})).toEqual({
await expect(
getFileCommitDate(path.join(repoDir, 'test.txt'), {}),
).resolves.toEqual({
date: new Date('2020-06-19'),
timestamp: new Date('2020-06-19').getTime() / 1000,
});
expect(getFileCommitDate(path.join(repoDir, 'dest.txt'), {})).toEqual({
await expect(
getFileCommitDate(path.join(repoDir, 'dest.txt'), {}),
).resolves.toEqual({
date: new Date('2020-09-13'),
timestamp: new Date('2020-09-13').getTime() / 1000,
});
});
it('returns latest commit date', async () => {
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'test.txt'), {age: 'newest'}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-09-13'),
timestamp: new Date('2020-09-13').getTime() / 1000,
});
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'dest.txt'), {age: 'newest'}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-11-13'),
timestamp: new Date('2020-11-13').getTime() / 1000,
});
});
it('returns latest commit date with author', async () => {
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'test.txt'), {
age: 'oldest',
includeAuthor: true,
}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-06-19'),
timestamp: new Date('2020-06-19').getTime() / 1000,
author: 'Caroline',
});
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'dest.txt'), {
age: 'oldest',
includeAuthor: true,
}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-09-13'),
timestamp: new Date('2020-09-13').getTime() / 1000,
author: 'Caroline',
});
});
it('returns earliest commit date with author', async () => {
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'test.txt'), {
age: 'newest',
includeAuthor: true,
}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-09-13'),
timestamp: new Date('2020-09-13').getTime() / 1000,
author: 'Caroline',
});
expect(
await expect(
getFileCommitDate(path.join(repoDir, 'dest.txt'), {
age: 'newest',
includeAuthor: true,
}),
).toEqual({
).resolves.toEqual({
date: new Date('2020-11-13'),
timestamp: new Date('2020-11-13').getTime() / 1000,
author: 'Josh-Cena',
});
});
it('throws custom error when file is not tracked', async () => {
expect(() =>
await expect(() =>
getFileCommitDate(path.join(repoDir, 'untracked.txt'), {
age: 'newest',
includeAuthor: true,
}),
).toThrow(FileNotTrackedError);
).rejects.toThrow(FileNotTrackedError);
});
it('throws when file not found', async () => {
expect(() =>
await expect(() =>
getFileCommitDate(path.join(repoDir, 'nonexistent.txt'), {
age: 'newest',
includeAuthor: true,
}),
).toThrow(
).rejects.toThrow(
/Failed to retrieve git history for ".*nonexistent.txt" because the file does not exist./,
);
});
Expand Down
40 changes: 27 additions & 13 deletions packages/docusaurus-utils/src/gitUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class FileNotTrackedError extends Error {}
* @throws Also throws when `git log` exited with non-zero, or when it outputs
* unexpected text.
*/
export function getFileCommitDate(
export async function getFileCommitDate(
/** Absolute path to the file. */
file: string,
args: {
Expand All @@ -36,12 +36,12 @@ export function getFileCommitDate(
/** Use `includeAuthor: true` to get the author information as well. */
includeAuthor?: false;
},
): {
): Promise<{
/** Relevant commit date. */
date: Date;
/** Timestamp in **seconds**, as returned from git. */
timestamp: number;
};
}>;
/**
* Fetches the git history of a file and returns a relevant commit date.
* It gets the commit date instead of author date so that amended commits
Expand All @@ -52,7 +52,7 @@ export function getFileCommitDate(
* @throws Also throws when `git log` exited with non-zero, or when it outputs
* unexpected text.
*/
export function getFileCommitDate(
export async function getFileCommitDate(
/** Absolute path to the file. */
file: string,
args: {
Expand All @@ -63,15 +63,16 @@ export function getFileCommitDate(
age?: 'oldest' | 'newest';
includeAuthor: true;
},
): {
): Promise<{
/** Relevant commit date. */
date: Date;
/** Timestamp in **seconds**, as returned from git. */
timestamp: number;
/** The author's name, as returned from git. */
author: string;
};
export function getFileCommitDate(
}>;

export async function getFileCommitDate(
file: string,
{
age = 'oldest',
Expand All @@ -80,11 +81,11 @@ export function getFileCommitDate(
age?: 'oldest' | 'newest';
includeAuthor?: boolean;
},
): {
): Promise<{
date: Date;
timestamp: number;
author?: string;
} {
}> {
if (!shell.which('git')) {
throw new GitNotFoundError(
`Failed to retrieve git history for "${file}" because git is not installed.`,
Expand All @@ -105,11 +106,24 @@ export function getFileCommitDate(
.filter(Boolean)
.join(' ');

const result = shell.exec(`git log ${args} -- "${path.basename(file)}"`, {
// Setting cwd is important, see: https://github.com/facebook/docusaurus/pull/5048
cwd: path.dirname(file),
silent: true,
const result = await new Promise<{
code: number;
stdout: string;
stderr: string;
}>((resolve) => {
shell.exec(
`git log ${args} -- "${path.basename(file)}"`,

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This string concatenation which depends on
library input
is later used in a
shell command
.
This string concatenation which depends on
library input
is later used in a
shell command
.
This string concatenation which depends on
library input
is later used in a
shell command
.
{
// Setting cwd is important, see: https://github.com/facebook/docusaurus/pull/5048
cwd: path.dirname(file),
silent: true,
},
(code, stdout, stderr) => {
resolve({code, stdout, stderr});
},
);
});

if (result.code !== 0) {
throw new Error(
`Failed to retrieve the git history for file "${file}" with exit code ${result.code}: ${result.stderr}`,
Expand Down

0 comments on commit 0da5877

Please sign in to comment.