Skip to content

Conversation

@CuriousLearner
Copy link
Owner

Summary

Add Vite bundling as a production-ready alternative to CDN-based asset delivery for the HTMX+Tailwind frontend option.
This addresses feedback that Keel relies on external CDNs in production, which is not ideal for production deployments.

Related Issues

Fixes #31

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Template configuration change
  • CI/CD update
  • Dependency update
  • Refactoring (no functional changes)

What Changed

When users select HTMX+Tailwind frontend, they now get a follow-up question to choose between:

  • Vite (default): Production-ready bundled assets with no external dependencies
  • CDN: Simple setup with no build step (existing behavior)

Template Changes

  • Added new Copier prompts
  • Modified existing prompts
  • Added conditional file generation
  • Modified project structure
  • Updated default values

Components Affected

  • Core template structure
  • Django settings
  • Authentication/Authorization
  • API (DRF/GraphQL)
  • Frontend (HTMX/Next.js)
  • Background tasks (Celery/Temporal)
  • Deployment configs (K8s/ECS/Fly.io/Render/EC2)
  • Observability (Sentry/OpenTelemetry/Prometheus)
  • SaaS features (Teams/Stripe/Feature flags)
  • Security configuration
  • Database/Cache configuration
  • Docker/Compose setup
  • CI/CD pipelines

Testing Checklist

  • Template generation succeeds without errors
  • Generated project structure is valid
  • All template tests pass (pytest)
  • Generated project tests pass (just test in generated project)
  • Docker builds successfully
  • Pre-commit hooks pass
  • Tested with multiple project types (if applicable)
  • Tested with different configuration combinations
  • Manual testing completed

Test Coverage

  • Current coverage: __%
  • Coverage change: __% (increase/decrease)

Breaking Changes

  • No breaking changes
  • Yes, breaking changes (documented below and in CHANGELOG with ⚠️)

Migration Guide:

Backward Compatibility

  • Fully backward compatible
  • Requires copier update with user action
  • Only affects new project generation
  • Requires manual intervention (describe below)

Security Considerations

  • No security impact
  • Security improvement
  • Potential security concern (describe below)

Removes reliance on external CDNs (cdn.tailwindcss.com, unpkg.com, jsdelivr.net) for production deployments when Vite option is selected.

Documentation

  • README updated
  • ReadTheDocs documentation updated
  • Inline code comments added/updated
  • CHANGELOG.md updated
  • Generated project docs updated
  • Copier prompt help text updated
  • No documentation needed

Deployment Impact

  • No deployment impact
  • Kubernetes manifests updated
  • Helm charts modified
  • Terraform configs changed
  • Ansible playbooks updated
  • Fly.io/Render configs modified
  • Docker/Compose files changed

Screenshots / Logs

Checklist

  • Code follows the project's style guidelines (ruff, mypy)
  • Self-review completed
  • Comments added for complex logic
  • No unnecessary debug code or comments
  • Conventional commit message format will be used
  • PR title clearly describes the change

Additional Notes


Reviewer Notes:

Add production-ready asset bundling as an alternative to CDN-based
delivery for the HTMX+Tailwind frontend option.

Changes:
- Add frontend_bundling copier question (vite/cdn, default: vite)
- Create Vite frontend setup (package.json, vite/tailwind/postcss configs)
- Add django-vite integration for asset loading
- Add vite service to docker-compose for development HMR
- Add multi-stage Dockerfile build for production assets
- Update .gitignore for node_modules and build artifacts

The CDN option remains available for quick prototypes, while Vite
provides bundled assets without external dependencies for production.
- Update CHANGELOG.md with frontend bundling option
- Update frontend-options.md with Vite vs CDN comparison
Copilot AI review requested due to automatic review settings November 29, 2025 21:14
Copilot finished reviewing on behalf of CuriousLearner November 29, 2025 21:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Vite bundling as a production-ready alternative to CDN-based asset delivery for projects using the HTMX+Tailwind frontend option. This addresses concerns about relying on external CDNs in production deployments by providing a local bundling option with no external dependencies.

Key Changes

  • Added frontend_bundling prompt to copier.yml with "vite" (default) and "cdn" options for HTMX+Tailwind projects
  • Implemented multi-stage Docker build for Vite asset bundling in production
  • Added Vite dev server as a separate service in docker-compose for local development

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
copier.yml Added frontend_bundling prompt conditional on htmx-tailwind selection
template/Dockerfile.jinja Added Node.js frontend-builder stage for building Vite assets
template/docker-compose.yml.jinja Added vite service for development with HMR support
template/config/settings/base.py.jinja Added django-vite configuration with dev server settings
template/.gitignore.jinja Added node_modules and static/dist exclusions for Vite
docs/features/frontend-options.md Documented Vite vs CDN bundling options with comparison table
CHANGELOG.md Added entry for new Vite bundling feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"dev_mode": DEBUG,
"manifest_path": BASE_DIR / "static" / "dist" / "manifest.json",
"static_url_prefix": "dist/",
"dev_server_host": "localhost",
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The dev_server_host is set to 'localhost', but in the docker-compose setup, the Vite service runs in a separate container. Django won't be able to reach the Vite dev server at 'localhost' when running in Docker. This should be set to 'vite' (the service name) to work correctly in the containerized development environment. Consider using an environment variable to support both local development (localhost) and Docker (vite).

Copilot uses AI. Check for mistakes.
- "5173:5173"
command: sh -c "npm install && npm run dev"
environment:
- VITE_DEV_SERVER_HOST=0.0.0.0
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The VITE_DEV_SERVER_HOST environment variable is set but never used. The vite.config.js hardcodes the host value to '0.0.0.0'. Either use this environment variable in vite.config.js or remove it to avoid confusion.

Suggested change
- VITE_DEV_SERVER_HOST=0.0.0.0

Copilot uses AI. Check for mistakes.
- Add frontend == 'htmx-tailwind' condition to Vite template files
- Update tests to check for Vite (default) and CDN modes separately
- Add new test_htmx_frontend_cdn_mode test
- Make VITE_DEV_SERVER_HOST configurable via environment variable
- Pass 'vite' hostname to Django web service in docker-compose
- Remove unused env var from vite service
- Defaults to 'localhost' for non-Docker development
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Copy rest of frontend
COPY frontend/ ./

# Build assets
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The build output directory should be explicitly created before running npm run build to ensure the parent /app/static directory exists. Add a line like RUN mkdir -p /app/static before the build command to prevent potential build failures if the directory doesn't exist in the builder stage.

Suggested change
# Build assets
# Build assets
RUN mkdir -p /app/static

Copilot uses AI. Check for mistakes.
]

THIRD_PARTY_APPS = [
{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The frontend_bundling variable is only defined when frontend == 'htmx-tailwind' (per copier.yml line 124). When other frontend options are selected, this variable will be undefined and referencing it will cause template rendering errors. Add a check: {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or use {% if frontend_bundling is defined and frontend_bundling == 'vite' -%} to safely handle cases where frontend_bundling is not set.

Suggested change
{% if frontend_bundling == 'vite' -%}
{% if frontend_bundling is defined and frontend_bundling == 'vite' -%}

Copilot uses AI. Check for mistakes.
Comment on lines +226 to +236
def test_htmx_frontend_cdn_mode(generate):
"""Test that HTMX frontend with CDN mode uses CDN links."""
project = generate(frontend="htmx-tailwind", frontend_bundling="cdn")

templates_dir = project / "templates"
base_html = (templates_dir / "base.html").read_text()
assert "{% block" in base_html
assert "tailwindcss" in base_html
assert "htmx" in base_html
# Should not have vite tags
assert "django_vite" not in base_html
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a test that verifies CDN mode doesn't create frontend files (package.json, vite.config.js, etc.) to ensure the conditional file generation works correctly in both directions.

Copilot uses AI. Check for mistakes.
MEDIA_URL = "/media/"
MEDIA_ROOT = BASE_DIR / "media"

{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or {% if frontend_bundling is defined and frontend_bundling == 'vite' -%}.

Suggested change
{% if frontend_bundling == 'vite' -%}
{% if frontend_bundling is defined and frontend_bundling == 'vite' -%}

Copilot uses AI. Check for mistakes.
/media
/staticfiles
/static_root
{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or {% if frontend_bundling is defined and frontend_bundling == 'vite' -%}.

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,23 @@
# syntax=docker/dockerfile:1
{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or {% if frontend_bundling is defined and frontend_bundling == 'vite' -%}.

Copilot uses AI. Check for mistakes.
{% if dependency_manager == 'uv' %}
- UV_NO_CACHE=1
{% endif %}
{% if frontend_bundling == 'vite' %}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' %} or {% if frontend_bundling is defined and frontend_bundling == 'vite' %}.

Suggested change
{% if frontend_bundling == 'vite' %}
{% if frontend_bundling is defined and frontend_bundling == 'vite' %}

Copilot uses AI. Check for mistakes.
ports:
- "1025:1025"
- "8025:8025"
{% if frontend_bundling == 'vite' %}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' %} or {% if frontend_bundling is defined and frontend_bundling == 'vite' %}.

Suggested change
{% if frontend_bundling == 'vite' %}
{% if frontend_bundling is defined and frontend_bundling == 'vite' %}

Copilot uses AI. Check for mistakes.
# Copy rest of application
COPY . .

{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or {% if frontend_bundling is defined and frontend_bundling == 'vite' -%}.

Copilot uses AI. Check for mistakes.
# Copy rest of application
COPY . .

{% if frontend_bundling == 'vite' -%}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

Same issue - frontend_bundling is undefined when frontend != 'htmx-tailwind'. Use {% if frontend == 'htmx-tailwind' and frontend_bundling == 'vite' -%} or {% if frontend_bundling is defined and frontend_bundling == 'vite' -%}.

Copilot uses AI. Check for mistakes.
@CuriousLearner CuriousLearner marked this pull request as draft November 29, 2025 21:36
- Add 'frontend == htmx-tailwind' check to all frontend_bundling conditionals
- Prevents template errors when frontend != htmx-tailwind
- Add mkdir -p /app/static before npm build in Dockerfile
- Enhance CDN mode test to verify no frontend files are created
- Add local development instructions (without Docker)
- Add production build commands
- Add troubleshooting table for common issues
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- ./static:/app/static
ports:
- "5173:5173"
command: sh -c "npm install && npm run dev"
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Consider using npm ci instead of npm install for more deterministic and faster installs in the development container. npm ci is designed for automated environments and provides reproducible builds.

Suggested change:

command: sh -c "npm ci && npm run dev"

This aligns with the production Dockerfile which uses npm ci (line 12).

Suggested change
command: sh -c "npm install && npm run dev"
command: sh -c "npm ci && npm run dev"

Copilot uses AI. Check for mistakes.

# Node
node_modules/
frontend/node_modules/
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The entry frontend/node_modules/ on line 52 is redundant since node_modules/ on line 51 already matches all node_modules directories recursively. Consider removing line 52 for simplicity.

Suggested change:

# Node
node_modules/
Suggested change
frontend/node_modules/

Copilot uses AI. Check for mistakes.
Comment on lines 206 to +224
def test_htmx_frontend_templates_generated(generate):
"""Test that HTMX frontend generates templates."""
"""Test that HTMX frontend generates templates with Vite (default)."""
project = generate(frontend="htmx-tailwind")

templates_dir = project / "templates"
assert (templates_dir / "base.html").exists()

base_html = (templates_dir / "base.html").read_text()
assert "{% block" in base_html
# Vite is the default, so check for vite tags
assert "django_vite" in base_html
assert "vite_asset" in base_html

# Vite frontend files should exist
frontend_dir = project / "frontend"
assert (frontend_dir / "package.json").exists()
assert (frontend_dir / "vite.config.js").exists()
assert (frontend_dir / "tailwind.config.js").exists()

Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Consider adding assertions to verify that django_vite is added to INSTALLED_APPS and that DJANGO_VITE settings are present in the generated settings file, similar to how test_drf_api_generated checks for DRF configuration. This would provide more comprehensive coverage.

Example additions:

# Check Django settings
settings = project / "config/settings/base.py"
settings_content = settings.read_text()
assert "django_vite" in settings_content
assert "DJANGO_VITE" in settings_content

Copilot uses AI. Check for mistakes.
- Use npm ci instead of npm install in docker-compose for deterministic installs
- Remove redundant frontend/node_modules/ from .gitignore
- Add settings assertions to Vite test for django_vite config
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


def test_htmx_frontend_cdn_mode(generate):
"""Test that HTMX frontend with CDN mode uses CDN links."""
project = generate(frontend="htmx-tailwind", frontend_bundling="cdn")
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The test test_all_features_enabled_generates_successfully in tests/test_features.py (line 390) uses frontend='htmx-tailwind' but doesn't specify frontend_bundling. Since Vite is the default, consider explicitly testing with frontend_bundling='vite' in that test or adding a comment explaining the default behavior is being tested.

Copilot uses AI. Check for mistakes.
COPY frontend/ ./

# Create output directory and build assets
RUN mkdir -p /app/static && npm run build
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The frontend builder creates /app/static but the COPY command on lines 66 and 91 copies from /app/static/dist to ./static/dist. The npm run build already outputs to ../static/dist per vite.config.js (line 8), so creating /app/static here is redundant. Consider removing mkdir -p /app/static && since Vite will create the necessary directories.

Suggested change
RUN mkdir -p /app/static && npm run build
RUN npm run build

Copilot uses AI. Check for mistakes.

| Issue | Solution |
|-------|----------|
| Vite HMR not working in Docker | Ensure `VITE_DEV_SERVER_HOST=vite` is set for the web service |
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The troubleshooting entry is slightly misleading. The VITE_DEV_SERVER_HOST environment variable should be set for the Django web service (as shown in docker-compose.yml.jinja line 31), not the Vite service. Consider clarifying this by stating 'Ensure VITE_DEV_SERVER_HOST=vite is set as an environment variable in the web service configuration'.

Suggested change
| Vite HMR not working in Docker | Ensure `VITE_DEV_SERVER_HOST=vite` is set for the web service |
| Vite HMR not working in Docker | Ensure `VITE_DEV_SERVER_HOST=vite` is set as an environment variable in the web service configuration |

Copilot uses AI. Check for mistakes.
- Explicitly test Vite in all-features test
- Remove redundant mkdir in Dockerfile (Vite creates dirs)
- Clarify troubleshooting doc for VITE_DEV_SERVER_HOST
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CuriousLearner CuriousLearner marked this pull request as ready for review November 30, 2025 20:01
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.

[Feature]: Add vite for FE assets bundling

2 participants