Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1914,6 +1914,42 @@ describe('getDirtyFiles', () => {
expect(pluginModule.getDirtyFiles('/some/dir')).toEqual(['M foo.js', '?? untracked.js']);
});

it('ignores the node_modules/package-lock.json that install itself created', () => {
mockExecFileSync.mockImplementation((cmd, args) => {
if (Array.isArray(args) && args[0] === 'rev-parse') return '.git\n';
return '?? node_modules/\n?? package-lock.json\n?? packages/alpha/node_modules/\n?? packages/alpha/package-lock.json\n';
});
expect(pluginModule.getDirtyFiles('/some/dir')).toEqual([]);
});

it('still reports user work that merely looks like an install artifact', () => {
mockExecFileSync.mockImplementation((cmd, args) => {
if (Array.isArray(args) && args[0] === 'rev-parse') return '.git\n';
return '?? node_modules_notes.md\n M src/package-lock.json.bak\n';
});
expect(pluginModule.getDirtyFiles('/some/dir')).toEqual(['?? node_modules_notes.md', 'M src/package-lock.json.bak']);
});

// Only `??` is npm's own output. Every tracked status at the same path is
// user work that updatePlugin would destroy, so it must keep blocking.
it.each([
[' M package-lock.json', 'M package-lock.json'],
['M package-lock.json', 'M package-lock.json'],
[' D package-lock.json', 'D package-lock.json'],
['D package-lock.json', 'D package-lock.json'],
['A package-lock.json', 'A package-lock.json'],
[' M packages/alpha/package-lock.json', 'M packages/alpha/package-lock.json'],
[' M node_modules/vendored/patch.js', 'M node_modules/vendored/patch.js'],
['R old-lock.json -> package-lock.json', 'R old-lock.json -> package-lock.json'],
['UU package-lock.json', 'UU package-lock.json'],
])('keeps tracked entry %j dirty', (porcelain, expected) => {
mockExecFileSync.mockImplementation((cmd, args) => {
if (Array.isArray(args) && args[0] === 'rev-parse') return '.git\n';
return `${porcelain}\n`;
});
expect(pluginModule.getDirtyFiles('/some/dir')).toEqual([expected]);
});

it('does not pass --untracked-files=no, so untracked files are reported (git already omits gitignored paths)', () => {
mockExecFileSync.mockImplementation((cmd, args) => {
expect(args).not.toContain('--untracked-files=no');
Expand Down
44 changes: 35 additions & 9 deletions src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,10 @@ function describeGitError(error: unknown): string {
/**
* Report tracked-file modifications and untracked files within `dir` in a git checkout.
*
* Untracked files are included on purpose: `git status` already excludes
* gitignored paths (build output like node_modules/dist never shows up), so
* anything untracked that does show up is real, unsaved user work — e.g. a
* new command file that hasn't been `git add`ed yet — which updating would
* destroy just as surely as an uncommitted edit to a tracked file.
* Untracked files are included on purpose: anything untracked is real, unsaved
* user work — e.g. a new command file that hasn't been `git add`ed yet — which
* updating would destroy just as surely as an uncommitted edit to a tracked
* file. The exception is `installArtifacts`: those are ours, not the user's.
*
* The `-- .` pathspec on `git status` restricts the report to `dir` itself.
* Without it, git reports the *entire enclosing repository* — e.g. a plugin
Expand Down Expand Up @@ -572,7 +571,9 @@ export function getDirtyFiles(dir: string): string[] {
encoding: 'utf-8',
stdio: ['pipe', 'pipe', 'pipe'],
});
return out.split('\n').map((line) => line.trim()).filter(Boolean);
return out.split('\n').filter((line) => line.trim())
.filter((line) => !isInstallArtifact(line))
.map((line) => line.trim());
} catch (error) {
throw new PluginError(
`Could not determine whether "${dir}" has uncommitted changes: git failed with: ${describeGitError(error)}`,
Expand All @@ -581,10 +582,35 @@ export function getDirtyFiles(dir: string): string[] {
}
}

/**
* Artifacts `installDependencies` creates by running `npm install` in the
* checkout, at the repo root and in every sub-plugin of a monorepo. A plugin
* repo without a .gitignore reports them as dirty, so without this the guard
* fires on webcmd's own output and every such plugin is permanently
* un-updatable — blaming the user for work they never did.
*/
const installArtifacts = /(?:^|\/)(?:node_modules(?:\/|$)|package-lock\.json$)/;

/**
* True only for an artifact npm itself created: a `??` (untracked) porcelain
* entry at an artifact path. The status columns are read from the raw line
* before any trimming, because every other status — ` M`, `M `, ` D`, `A `,
* `R `, `UU` — is tracked work the user could lose when `updatePlugin`
* replaces the directory, no matter what the path looks like.
*/
function isInstallArtifact(line: string): boolean {
if (line.slice(0, 2) !== '??') return false;
return installArtifacts.test(line.slice(2).trim());
}

/** Path portion of a `git status --porcelain` entry (already trimmed of its leading space). */
function dirtyEntryPath(entry: string): string {
return entry.startsWith('??') ? entry.slice(2).trim() : entry.replace(/^[MADRCU!]{1,2}\s+/, '');
}

function describeDirtyEntry(entry: string): string {
const isUntracked = entry.startsWith('??');
const file = entry.replace(/^\?\?\s*/, '').replace(/^[MADRCU! ]+\s*/, '');
return isUntracked ? `${file} (new, unstaged)` : `${file} (modified)`;
const file = dirtyEntryPath(entry);
return entry.startsWith('??') ? `${file} (new, unstaged)` : `${file} (modified)`;
}

function assertPluginNotDirty(name: string, dir: string, force: boolean): void {
Expand Down