Use spawnSync instead of execaSync in git.ts

[kevinzunigacuellar]

Description

• Closes ESM errors when building. #887
• Replace execa with node:child_process

[changeset-bot]

🦋 Changeset detected

Latest commit: d1fcaef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@astrojs/starlight	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
starlight	✅ Ready (Inspect)	Visit Preview	Jan 9, 2024 7:14pm

1 Ignored Deployment

Name	Status	Preview	Updated (UTC)
starlight-i18n	⬜️ Ignored (Inspect)	Visit Preview	Jan 9, 2024 7:14pm

[astrobot-houston]

size-limit report 📦

Path	Size
/index.html	5.21 KB (0%)
/_astro/*.js	21.42 KB (0%)
/_astro/*.css	13.04 KB (0%)

[HiDeoo]

This is super cool, I remember starting something similar a while back but never finished it. Glad to see someone tackling this 🙌

A few notes:

• I'm not sure this should spawn a new shell so I think spawnSync (which accepts a encoding: 'utf-8' option to get a string back in result.stdout) might be a better choice and would also not change the current behavior.

• execa should be removed.

• Maybe we could take this opportunity to remove the oldest code path as we do not ever use it (and maybe rename the function? I remember using getFileNewestCommitDate at the time) and also remove FileNotTrackedError and replace it by a basic Error?

• I remember starting to work on a git.test.ts file for this as we started to own a bit more this piece of code. If you think this could be useful, I'm sharing the WIP here in case it could help (this assume git is installed on the machine running the tests but I think this is a fair assumption?)

  import { mkdtempSync, writeFileSync } from 'node:fs';
  import { join } from 'node:path';
  import { tmpdir } from 'node:os';
  import { spawnSync } from 'node:child_process';
  import { describe, expect, test } from 'vitest';
  import { getFileNewestCommitDate } from '../../utils/git';
  
  describe('getFileNewestCommitDate', () => {
    const { commitAllChanges, getFilePath, writeFile } = makeTestRepo();
  
    test('returns the newest commit date', () => {
      const file = 'updated.md';
      const lastCommitDate = '2023-06-25';
  
      writeFile(file, 'content 0');
      commitAllChanges('add updated.md', '2023-06-21');
      writeFile(file, 'content 1');
      commitAllChanges('update updated.md', lastCommitDate);
  
      expectCommitDateToEqual(getFileNewestCommitDate(getFilePath(file)), lastCommitDate);
    });
  
    test('returns the initial commit date for a file never updated', () => {
      const file = 'added.md';
      const commitDate = '2022-09-18';
  
      writeFile(file, 'content');
      commitAllChanges('add added.md', commitDate);
  
      expectCommitDateToEqual(getFileNewestCommitDate(getFilePath(file)), commitDate);
    });
  
    test('returns the newest commit date for a file with a name that contains a space', () => {
      const file = 'updated with space.md';
      const lastCommitDate = '2021-01-02';
  
      writeFile(file, 'content 0');
      commitAllChanges('add updated.md', '2021-01-01');
      writeFile(file, 'content 1');
      commitAllChanges('update updated.md', lastCommitDate);
  
      expectCommitDateToEqual(getFileNewestCommitDate(getFilePath(file)), lastCommitDate);
    });
  
    test('returns the newest commit date for a file updated the same day', () => {
      const file = 'updated-same-day.md';
      const lastCommitDate = '2023-06-25T14:22:35Z';
  
      writeFile(file, 'content 0');
      commitAllChanges('add updated.md', '2023-06-25T12:34:56Z');
      writeFile(file, 'content 1');
      commitAllChanges('update updated.md', lastCommitDate);
  
      expectCommitDateToEqual(getFileNewestCommitDate(getFilePath(file)), lastCommitDate);
    });
  
    test('throws when failing to retrieve the git history for a file', () => {
      expect(() =>
        getFileNewestCommitDate(getFilePath('../not-a-starlight-test-repo/test.md'))
      ).toThrow(/^Failed to retrieve the git history for file "[/\\-\w ]+\/test\.md" with exit code/);
    });
  
    test('throws when trying to get the history of a non-existing or untracked file', () => {
      const expectedError =
        /^Failed to retrieve the git history for file "[/\\-\w ]+\/(?:unknown|untracked)\.md" because the file is not tracked by git\.$/;
  
      writeFile('untracked.md', 'content');
  
      expect(() => getFileNewestCommitDate(getFilePath('unknown.md'))).toThrow(expectedError);
      expect(() => getFileNewestCommitDate(getFilePath('untracked.md'))).toThrow(expectedError);
    });
  });
  
  function expectCommitDateToEqual(commitDate: CommitDate, expectedDateStr: ISODate) {
    const expectedDate = new Date(expectedDateStr);
  
    expect(commitDate).toStrictEqual({
      date: expectedDate,
      timestamp: expectedDate.getTime() / 1000,
    });
  }
  
  function makeTestRepo() {
    const repoPath = mkdtempSync(join(tmpdir(), 'starlight-test-git-'));
  
    function runInRepo(command: string, args: string[], env: NodeJS.ProcessEnv = {}) {
      const result = spawnSync(command, args, { cwd: repoPath, env });
  
      if (result.status !== 0) {
        throw new Error(`Failed to execute test repository command: '${command} ${args.join(' ')}'`);
      }
    }
  
    // Configure git specifically for this test repository.
    runInRepo('git', ['init']);
    runInRepo('git', ['config', 'user.name', 'starlight-test']);
    runInRepo('git', ['config', 'user.email', 'starlight-test@example.com']);
    runInRepo('git', ['config', 'commit.gpgsign', 'false']);
  
    return {
      // The `dateStr` argument should be in the `YYYY-MM-DD` or `YYYY-MM-DDTHH:MM:SSZ` format.
      commitAllChanges(message: string, dateStr: ISODate) {
        const date = dateStr.endsWith('Z') ? dateStr : `${dateStr}T00:00:00Z`;
  
        runInRepo('git', ['add', '-A']);
        // This sets both the author and committer dates to the provided date.
        runInRepo('git', ['commit', '-m', message, '--date', date], { GIT_COMMITTER_DATE: date });
      },
      getFilePath(name: string) {
        return join(repoPath, name);
      },
      writeFile(name: string, content: string) {
        writeFileSync(join(repoPath, name), content);
      },
    };
  }
  
  type ISODate =
    | `${number}-${number}-${number}`
    | `${number}-${number}-${number}T${number}:${number}:${number}Z`;
  
  type CommitDate = ReturnType<typeof getFileNewestCommitDate>;

[kevinzunigacuellar]

Thanks for the review Hideo 💜 . I added most of your suggested changes, and cleaned up the logic a bit. However, I seem to have broken a test 😆, which I am not sure how to fix. It seems that my root file is pointing to the test directory but every time I try changing root path in the test-config I break the entire thing 💥. Do you have any ideas?

[HiDeoo]

I don't have a computer to actually run the code but from the changes, I would say that reverting the changes to getLastUpdated() (minus the name and argument changes to getFileCommitDate() ofc) should fix the issue as you removed the try-catch block that we would still need to ignore the error and I think that in this test, the path is invalid as it's based on the actual docs (that's also one of the reason I started working on the dedicated tests I linked earlier so we can easily test the git part in isolation).

[kevinzunigacuellar]

Thank you Hideo, it was the error catch 😄 . I was able to add your tests in git.test.ts 💜

[delucis]

Woo this looks great. Thanks @kevinzunigacuellar and thank you @HiDeoo for your very insightful review 💖

Left one small refactor suggestion, but overall, the code looks good to me.

One thought: have we tested this on Windows? We don’t currently use the last-updated feature on the docs site and building the Starlight docs is the only “test” we currently run on Windows. Would be good to be sure that’s working.

[lilnasy]

Haven't looked at the code but the result looks great to me!

steps chris told me to follow

result

[hippotastic]

Also works well for me on Windows 10!

[at-the-vr]

Working for me as well on Win 10, I went across the translations found no Issues. Found the locale translation interesting and that's without any hardcoding 🔥🔥

Locale	Last Updated Image
Deutsch
Japanese & Korean

There are 13 more languages but these 3 caught my attention 😅. Overall it works 😄

[delucis]

Let’s go! Thanks again @kevinzunigacuellar and thanks everyone for testing this out 💖