Skip to content

Conversation

@jchris
Copy link
Collaborator

@jchris jchris commented Aug 10, 2025

Complete rewrite: self-sufficient build/test system for the Vite template package.

Context

This addresses all review feedback by removing assumptions about pre-existing template/ content and making the build + tests deterministic and CI-safe. The template is now assembled from a single source of truth in the repo and fully validated end‑to‑end via npm pack.

Changes

  • build-template.sh
    • Fail-fast shell: set -Eeuo pipefail and safe IFS.
    • Fresh build: remove any existing template/ to avoid stale files.
    • Single source of truth: copy from repo root src/, public/ (if present), and index.html into template/.
    • Null-safe config copy: loop with nullglob over tsconfig*.json, vite.config.*, *.config.*, tailwind.config.*.
    • Generate template/_package.json with correct name eject-vibe, scripts (dev, build, preview, format), and caret dependency ranges aligned with Vite 7 + React 19 + Tailwind v4.
    • Add template/.prettierignore to exclude dist, node_modules, and lockfile from format checks.
  • test-template-pack.sh
    • Deterministic tarball capture: read npm pack output; no ls globbing.
    • Robust quoting and trap cleanup using a mktemp workdir.
    • Assertions verify template/_package.json, template/src/App.tsx, and template/index.html exist, then install, typecheck, build, and simulate an eject edit.
  • package.json (root)
    • check: CI-safe—builds template, installs deps in template/ without requiring a lockfile, builds, runs tsc --noEmit, runs prettier --check, and cleans up temp files (package.json, lockfile, node_modules, dist).
    • build-template/test-local: use bash and align with the new scripts.
  • Source-of-truth app files restored under repo root
    • Added minimal React+TS+Tailwind app in src/ and linked CSS via <link rel="stylesheet" href="/src/index.css"> to avoid side-effect imports under noUncheckedSideEffectImports.
    • Restored public/vite.svg referenced by index.html.

Template manifest (generated)

Key fields in template/_package.json:

  • name: eject-vibe
  • scripts: dev, build (tsc -b && vite build), preview, format (prettier --check .)
  • deps: react, react-dom
  • devDeps: vite, @vitejs/plugin-react, typescript, tailwindcss, @tailwindcss/postcss, postcss, autoprefixer, prettier, @types/* (caret ranges)

Packaging hygiene

  • Root files: ["template"] ensures only the intended contents are published.
  • .gitignore already excludes template/ and test artifacts; no extra ignores needed.

Impact

  • Fresh clone works: npm run build-template produces a complete template/ with src/, public/, index.html, configs, and _package.json.
  • CI-safe checks: npm run check installs/builds/types/format-checks without requiring a pre-existing lockfile and cleans temporary artifacts.
  • Deterministic E2E validation: npm run test-local packs the repo and validates the published contents and build.

Verification

$ npm run -s check
→ success (build ok, vite build ok, tsc ok, prettier check ok)

$ npm run -s test-local
→ success (tarball: create-vite-eject-vibe-1.0.0.tgz; assertions passed; build ok)

If you’d like any tweaks to the dependency ranges or script names, I can adjust quickly.

@charliecreates charliecreates bot requested a review from CharlieHelps August 10, 2025 23:46
Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • The build script relies on a pre-existing template/src while the repo deletes sources and ignores template/, causing fatal failures on clean clones/CI. The build must populate template/ with src/, public/, and index.html.
  • Fragile glob copies (cp *.config.*) with set -e will fail when no matches exist; make these null-safe.
  • npm run check uses npm ci without a lockfile inside template/, which will fail; switch to npm install or provide a lockfile.
  • Minor robustness issues in test packaging (capture tarball name from npm pack, add cleanup trap) and overly tight devDependency pinning in the generated template.
Additional notes (1)
  • Maintainability | test-template-pack.sh:7-14
    Consider adding a cleanup trap so temporary files are removed even if the test fails midway. This keeps the workspace clean and avoids interference between runs.
Summary of changes
  • Added a build script build-template.sh that prepares a template/ directory, copies config files, and generates template/_package.json plus an empty template/dist/.
  • Converted the project into an npm-publishable Vite template package by updating package.json (name, metadata, scripts, files field) and adding test scripts (build-template and test-local).
  • Added a test script test-template-pack.sh that packs the repo, verifies the packaged contents, installs template deps, builds, and validates an eject workflow.
  • Updated .gitignore to ignore test-output/, generated tarballs, and the generated template/ directory.
  • Removed previous app source files under src/ and public/ to rely on the generated template/.

Comment on lines 10 to 16
echo "📋 Source files already in template/ - preserving them..."
# Source files (src/, public/, index.html) are already in template/
# Just ensure directory exists
if [[ ! -d "template/src" ]]; then
echo "❌ ERROR: template/src not found"
exit 1
fi

Choose a reason for hiding this comment

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

This script assumes template/src already exists and explicitly exits if it does not. However, the repo deletes src/ and public/ and also ignores template/ via .gitignore. On a clean clone or CI runner, template/src will not exist, so this will always fail. This contradicts the PR summary stating the template is built from source files and eliminates manual sync. You need to copy or generate the app sources (src/, public/, and index.html) into template/ during the build.

Suggestion

Change the build to actually populate template/ from your single source of truth, and remove the hard dependency on pre-existing template/src. For example:

# after creating template directories
rsync -a --delete src/ template/src/
rsync -a --delete public/ template/public/
cp -f index.html template/

Alternatively, if the source-of-truth lives elsewhere, point these commands there. Also remove the early exit block that errors on missing template/src. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

Comment on lines 19 to 21
cp tsconfig*.json template/
cp *.config.* template/
cp vite.config.ts template/

Choose a reason for hiding this comment

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

Using bare globs with set -e makes the script brittle. If no files match *.config.* (or even tsconfig*.json in some repos), cp will error and the script will exit. This causes false failures depending on the presence of optional configs.

Suggestion

Make glob copies null-safe. For example:

shopt -s nullglob
for f in tsconfig*.json vite.config.* *.config.*; do
  [ -e "$f" ] && cp "$f" template/
done
shopt -u nullglob

This avoids failing when optional config files are absent. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

@@ -0,0 +1,66 @@
#!/bin/bash
set -e

Choose a reason for hiding this comment

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

The script currently uses only set -e. Adding -Euo pipefail improves robustness by catching unset variables and pipeline failures, which is especially useful in packaging/build scripts.

Suggestion

Strengthen the shell safety settings:

set -Eeuo pipefail

This makes the build more resilient to subtle failures. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

package.json Outdated
"files": ["template"],
"scripts": {
"build-template": "./build-template.sh",
"check": "./build-template.sh && cd template && cp _package.json package.json && npm ci && npm run build && npx tsc --noEmit && npm run format --check && rm package.json",

Choose a reason for hiding this comment

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

check uses npm ci inside template/ right after creating package.json. Without a package-lock.json in template/, npm ci will fail by design. Your test-template-pack.sh uses npm install, which is correct here. The mismatch will break npm run check.

Suggestion

Either switch to npm install in check, or ensure a lockfile exists before running npm ci (e.g., copy a curated package-lock.json into template/). Recommended simple fix:

"check": "./build-template.sh && cd template && cp _package.json package.json && npm install --no-audit --no-fund && npm run build && npx tsc --noEmit && npm run format --check && rm package.json"

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

Comment on lines 15 to 18
echo "📦 Creating npm package..."
npm pack ..
PACKAGE_FILE=$(ls create-vite-eject-vibe-*.tgz)
echo "✅ Created: $PACKAGE_FILE"

Choose a reason for hiding this comment

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

Capturing the packed tarball name via ls is fragile (multiple files, locale differences). npm pack already prints the archive name—capture that directly to avoid ambiguity.

Suggestion

Capture the output of npm pack directly:

PACKAGE_FILE=$(npm pack .. | tail -n1)
echo "✅ Created: $PACKAGE_FILE"

Then use it with quotes: tar -xzf "$PACKAGE_FILE". Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.

Comment on lines 37 to 45
if [[ ! -f "template/src/App.tsx" ]]; then
echo "❌ ERROR: template/src/App.tsx missing"
exit 1
fi

if [[ ! -f "template/index.html" ]]; then
echo "❌ ERROR: template/index.html missing"
exit 1
fi

Choose a reason for hiding this comment

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

The test checks for template/src/App.tsx and template/index.html, but the build script never creates or copies them. The current test-local npm script runs the build first, but the build must be responsible for populating these files for a fresh clone/CI. This is currently a broken contract between the build and test steps.

Suggestion

After fixing build-template.sh to populate template/src, template/public, and template/index.html, keep these assertions. If you do not intend to generate sources during build, adjust the test to create the required files before packing—but that defeats the stated goal of automation. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with the coordinated fixes.

Comment on lines 49 to 52
'prettier': '^3.6.2',
'tailwindcss': '^4.0.0',
'typescript': '~5.8.3',
'vite': '^7.1.0'

Choose a reason for hiding this comment

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

The devDependency versions in the generated template are very tightly pinned (e.g., "typescript": "~5.8.3"). This increases the risk of install failures if that exact patch is unavailable or unpublished in some registries/mirrors. Templates typically use caret ranges aligned with Vite's official templates to balance stability and availability.

Suggestion

Loosen version ranges to stable caret ranges consistent with Vite templates, e.g. "typescript": "^5.8.0" and similar for related tools. Optionally, source these versions from a single versions map to keep them consistent. Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating the ranges.

@charliecreates charliecreates bot removed the request for review from CharlieHelps August 10, 2025 23:47
@jchris
Copy link
Collaborator Author

jchris commented Aug 10, 2025

@CharlieHelps feel free to do a complete rewrite, please update the PR

@charliecreates
Copy link

charliecreates bot commented Aug 10, 2025

Expand this to see my work.
  • Inspected build-template.sh, test-template-pack.sh, root package.json, and .gitignore for all legacy logic, directory references, and copy assumptions.
  • Audited src/, public/, and index.html to define the single source-of-truth for app scaffolding and config.
  • Checked all scripts for CI compliance; identified that legacy builds abort if template/src is missing.
  • Gathered current dependency versions and verified alignment with Vite template conventions.
  • Finished rewriting build-template.sh to always regenerate template/ from root-level sources, hardening with set -Eeuo pipefail, null-safe config copying, and clear errors for required files.
  • Integrated deterministic, semver-aligned template _package.json output under template/, validated for scripts and dependency ranges.
  • Rewrote test-template-pack.sh for deterministic tarball detection, robust cleanup, and validated assertions on generated package structure (see latest script patch).
  • Updated package.json scripts to provide a CI-compatible check phase and enforce cleanup after checks, removing install/lockfile assumptions.
  • Added root src/main.tsx, src/App.tsx, src/index.css as the canonical app scaffolding; injected /src/index.css into index.html for clarity.
  • Committed, pushed, rewrote PR #1 with full context and verification, and requested review by jchris.

- build-template.sh: populate template/ from root src/, public/, index.html; null-safe copying; strict shell flags; generate _package.json with sane caret ranges; add .prettierignore
- test-template-pack.sh: deterministic npm pack capture, robust quoting, cleanup trap, end-to-end checks
- package.json: CI-safe check script (no lockfile dependency), cleanups; align scripts
- add source-of-truth app in src/ and public/vite.svg; link CSS via index.html to satisfy TS noUncheckedSideEffectImports

Passes local check and pack tests.
@charliecreates
Copy link

Pushed a complete rewrite of the build/test system and updated the PR body. The template now builds from a single source of truth and validates end‑to‑end in CI.

What changed
  • build-template.sh

    • Uses set -Eeuo pipefail and safe IFS.
    • Rebuilds template/ from scratch: copies root src/, public/ (if present), and index.html.
    • Null‑safe config copy for tsconfig*.json, vite.config., .config., tailwind.config..
    • Generates template/_package.json with correct name eject-vibe, scripts (dev, build, preview, format), and caret ranges aligned with Vite 7 + React 19 + Tailwind v4.
    • Adds template/.prettierignore (dist, node_modules, package-lock.json).
  • test-template-pack.sh

    • Captures the npm pack tarball deterministically (no ls globbing).
    • Uses mktemp workdir + trap cleanup and consistent quoting.
    • Verifies required files, installs deps, runs tsc, builds, and simulates an eject edit.
  • package.json (root)

    • check script is CI‑safe: builds the template, npm install (no lockfile required), vite build, tsc --noEmit, prettier --check, then cleans package.json/lockfile/node_modules/dist inside template/.
    • build-template/test-local scripts use bash.
  • Source of truth restored

    • Minimal React+TS+Tailwind app added under src/.
    • index.html links /src/index.css to avoid side‑effect CSS imports under noUncheckedSideEffectImports.
    • public/vite.svg restored to match index.html.
  • Packaging hygiene

    • files: ["template"] ensures only intended contents are published.

Verification

  • Commands run locally:
    • npm run -s check → success
    • npm run -s test-local → success (tarball: create-vite-eject-vibe-1.0.0.tgz; assertions passed; build ok)

If you want different dependency ranges or script names, I can adjust quickly.

jchris and others added 2 commits August 10, 2025 17:00
- Fixes CI error where npx tsc was resolving to wrong package
- Now uses ./node_modules/.bin/tsc to ensure correct TypeScript compiler

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Replace direct tsc/build commands with npm run check
- Use npm run test-local for comprehensive template testing
- Rename jobs to reflect new template package structure
- Fixes CI TypeScript compilation error

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@jchris
Copy link
Collaborator Author

jchris commented Aug 11, 2025

@CharlieHelps check the ci coverage

@charliecreates
Copy link

charliecreates bot commented Aug 11, 2025

Expand this to see my work.
  • Reviewed .github/workflows/ci.yml and confirmed there are no steps that run tests with coverage, nor any steps that upload coverage artifacts or annotate results.
  • Inspected package.json to confirm that the check and test-local scripts call shell scripts for build and manual checks, but do not invoke test frameworks or generate any coverage reports.
  • Thoroughly searched the repo for common test or coverage tooling—no test framework dependencies or configs found in package.json, nor any coverage-related files or settings.
  • Examined build-template.sh and test-template-pack.sh scripts to confirm they perform build and packaging checks without executing tests or generating coverage metrics.
  • Checked the full repo for test files or specs—found none under src/ or elsewhere; no tests directory or coverage config exists.
  • Reviewed the latest CI run for this PR; all jobs completed successfully, but no coverage metrics are printed in logs nor are any coverage artifacts present.
  • Posted a detailed comment to PR #1 summarizing that CI does not currently collect or report coverage and asking for clarification on desired test and coverage tooling or any specific standards or services to be used.

@charliecreates
Copy link

Coverage status: CI is not collecting or reporting code coverage for this PR.

  • Latest run: CI passed on 2025-08-11 00:04 UTC across 3 jobs (template-package-test; template-check on Node 20 and 22). No coverage summary in logs and no artifacts were uploaded. See run: https://github.com/VibesDIY/eject-vibe/actions/runs/16867871803
  • Workflow: .github/workflows/ci.yml runs npm ci + npm run check, and npm run test-local. There are no coverage flags, no upload steps (e.g., lcov upload, Codecov/Coveralls), and no job annotations for coverage.
  • Scripts: package.json defines:
    • check → builds the template, installs deps, vite build, tsc --noEmit, prettier --check. No tests or coverage.
    • test-local → builds the template and runs test-template-pack.sh (pack + install + typecheck + build + simple eject check). No test runner, no coverage.
  • Tests in repo: none detected (no Jest/Vitest configs, no .test. files). There’s nothing that would generate coverage even locally.

Open questions

  1. Do you want coverage for the generated template app itself (e.g., Vitest + V8 coverage run inside template/) or for the packaging scripts, or both?
  2. Preferred tooling: Vitest coverage (c8/V8) vs Jest/istanbul? Any minimum thresholds?
  3. Reporting: GitHub job summary only, or integrate Codecov/Coveralls? If external, which org/repo settings should we use?
  4. Scope/matrix: collect coverage on a single Node version (e.g., 20) or across the matrix?

If you confirm the targets and tool, I can add a follow-up PR that introduces a minimal test suite in the template, enables coverage (e.g., vitest --coverage), and uploads an lcov artifact and summary in CI.

@jchris
Copy link
Collaborator Author

jchris commented Aug 11, 2025

@CharlieHelps yes just the hello world of test suites, for best practices so we are setup for success

@charliecreates
Copy link

charliecreates bot commented Aug 11, 2025

Expand this to see my work.
  • Scoped all required changes for integrating a minimal test setup tied to template/_package.json, root package.json, build-template.sh, and .github/workflows/ci.yml.
  • Added vitest.config.ts as a standalone config for Vitest, ensured use of a jsdom test environment and project-consistent devDependencies.
  • Created a trivial src/App.test.tsx validating test run with Vitest and Testing Library, targeting template app functionality.
  • Updated build-template.sh to ensure test files/configs and dependencies flow into the built template for a clean post-build experience.
  • Patched template/_package.json to include test/test:watch scripts and testing dependencies matching caret range conventions.
  • Modified root package.json to extend npm run check so it installs deps and runs the test suite within template/ as part of the standard flow.
  • Ensured test steps run during CI by updating .github/workflows/ci.yml, keeping minimal 'hello world' coverage for PR workflows.
  • Addressed Prettier warnings by updating src/App.test.tsx and vitest.config.ts for semicolon and bracket consistency.
  • Validated that npm run check and npm run test-local both pass fully, running the sample suite and confirming no regressions or artifacts.
  • Committed and pushed all test and script integration changes to the vite-template-clean branch for PR Implement Vite template package with automated build system #1.

… Add Vitest config (vitest.config.ts) and jsdom env\n- Generate test deps/scripts in template _package.json\n- Add src/App.test.tsx (hello-world component test)\n- Run tests in root check script and pack test\n- Keep Prettier passing and CI-safe
@charliecreates
Copy link

Added a minimal test suite to the template and wired it into scripts and CI.

What changed

  • Template tests
    • Added src/App.test.tsx (renders and asserts the heading).
    • Added vitest.config.ts with jsdom environment.
  • Template package generation
    • build-template.sh now includes testing deps and scripts in template/_package.json:
      • devDependencies: vitest, jsdom, @testing-library/react
      • scripts: test (vitest run), test:watch (vitest)
    • vitest.config.ts is copied into template/ by existing null-safe glob (.config.).
  • Root scripts and pack test
    • package.json check now runs the template test suite after install.
    • test-template-pack.sh runs npm test inside the packed template before build/typecheck.
  • No coverage added (kept to “hello world”). Happy to add later.

Why this fits

  • Uses Vitest + jsdom + Testing Library, the standard minimal stack for Vite + React + TS.
  • Tests live under src so they’re included in template/ by the build script.
  • CI already calls npm run check, which now runs the tests; the pack job also exercises tests.

Verification

  • Local (fresh build of template, install, test, build, typecheck, format):
    • npm run -s check → Test Files: 1 passed; Vite build ok; tsc ok; Prettier ok
  • Pack flow (validates published tarball):
    • npm run -s test-local → Test Files: 1 passed; Template structure ok; Build ok; Eject edit ok

Notes

  • No changes required to .github/workflows/ci.yml; tests now run as part of “Check template” and the pack job.
  • If you want coverage (vitest --coverage + artifact upload) I can add it in a small follow-up.

@jchris jchris merged commit e96e2f5 into main Aug 11, 2025
3 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.

3 participants