Skip to content

feat: add /dist/ to .gitignore during vinext init#220

Merged
southpolesteve merged 2 commits intocloudflare:mainfrom
vikas5914:feat/init-gitignore
Mar 3, 2026
Merged

feat: add /dist/ to .gitignore during vinext init#220
southpolesteve merged 2 commits intocloudflare:mainfrom
vikas5914:feat/init-gitignore

Conversation

@vikas5914
Copy link
Copy Markdown
Contributor

Summary

  • vinext init now adds /dist/ to .gitignore (Step 6 in the init flow)
  • Creates .gitignore if it doesn't exist, appends the entry if missing, skips if already present
  • Preserves all existing .gitignore entries

Changes

  • packages/vinext/src/init.ts: Added updateGitignore() function, wired into init() as Step 6, added updatedGitignore to InitResult, added summary log line
  • tests/init.test.ts: Added 8 new tests — 5 unit tests for updateGitignore() and 3 integration tests for the full init flow

Why

vite build outputs to /dist/ by default. Without this entry in .gitignore, users who run vinext init and then build will end up with build artifacts tracked in git.

vinext build outputs to /dist/ which should not be committed.
The init command now ensures /dist/ is in .gitignore, creating
the file if needed and skipping if the entry already exists.
@southpolesteve
Copy link
Copy Markdown
Collaborator

/bigbonk review this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Mar 2, 2026

Open in StackBlitz

npm i https://pkg.pr.new/vinext@220

commit: ac85ea2

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean implementation — the function is well-structured, tests are thorough, and it integrates naturally into the init flow. One edge case worth considering and a couple minor notes below.

Comment thread packages/vinext/src/init.ts Outdated
Comment thread tests/init.test.ts
Comment thread packages/vinext/src/init.ts Outdated
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Mar 2, 2026

Review posted. Here's a summary of my feedback:

Main concern: The duplicate detection only checks for the exact string /dist/ (with leading slash). Many projects — including this repo's own .gitignore — use dist/ (without the leading slash), which is actually a broader pattern. If a user already has dist/ in their .gitignore, updateGitignore would add a redundant /dist/ entry. I suggested broadening the check to also recognize dist/ and dist as already covering the dist directory.

Minor notes:

  • Suggested adding a test case for the dist/ (no leading slash) variant
  • Small naming nit on the entry variable if the broader matching is adopted

Otherwise the PR is clean: the function handles file creation, appending, no-trailing-newline, and idempotency correctly. Tests are comprehensive with both unit and integration coverage, and all 54 tests pass.

github run

Address review feedback: detect dist/ (no leading slash) and bare dist
as existing coverage for the dist directory to avoid adding a redundant
/dist/ entry. Rename entry -> exactEntry for clarity.
@vikas5914
Copy link
Copy Markdown
Contributor Author

@southpolesteve updated RR with changes from bigbonk.

@southpolesteve southpolesteve enabled auto-merge (squash) March 3, 2026 16:16
@southpolesteve southpolesteve merged commit 985e499 into cloudflare:main Mar 3, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants