-
-
Notifications
You must be signed in to change notification settings - Fork 26
feat(frontend): add Vite bundling option for HTMX+Tailwind #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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
There was a problem hiding this 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_bundlingprompt 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", |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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).
template/docker-compose.yml.jinja
Outdated
| - "5173:5173" | ||
| command: sh -c "npm install && npm run dev" | ||
| environment: | ||
| - VITE_DEV_SERVER_HOST=0.0.0.0 |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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.
| - VITE_DEV_SERVER_HOST=0.0.0.0 |
- 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
There was a problem hiding this 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.
template/Dockerfile.jinja
Outdated
| # Copy rest of frontend | ||
| COPY frontend/ ./ | ||
|
|
||
| # Build assets |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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.
| # Build assets | |
| # Build assets | |
| RUN mkdir -p /app/static |
| ] | ||
|
|
||
| THIRD_PARTY_APPS = [ | ||
| {% if frontend_bundling == 'vite' -%} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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.
| {% if frontend_bundling == 'vite' -%} | |
| {% if frontend_bundling is defined and frontend_bundling == 'vite' -%} |
| 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 |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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.
| MEDIA_URL = "/media/" | ||
| MEDIA_ROOT = BASE_DIR / "media" | ||
|
|
||
| {% if frontend_bundling == 'vite' -%} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' -%}.
| {% if frontend_bundling == 'vite' -%} | |
| {% if frontend_bundling is defined and frontend_bundling == 'vite' -%} |
template/.gitignore.jinja
Outdated
| /media | ||
| /staticfiles | ||
| /static_root | ||
| {% if frontend_bundling == 'vite' -%} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' -%}.
template/Dockerfile.jinja
Outdated
| @@ -1,4 +1,23 @@ | |||
| # syntax=docker/dockerfile:1 | |||
| {% if frontend_bundling == 'vite' -%} | |||
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' -%}.
template/docker-compose.yml.jinja
Outdated
| {% if dependency_manager == 'uv' %} | ||
| - UV_NO_CACHE=1 | ||
| {% endif %} | ||
| {% if frontend_bundling == 'vite' %} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' %}.
| {% if frontend_bundling == 'vite' %} | |
| {% if frontend_bundling is defined and frontend_bundling == 'vite' %} |
template/docker-compose.yml.jinja
Outdated
| ports: | ||
| - "1025:1025" | ||
| - "8025:8025" | ||
| {% if frontend_bundling == 'vite' %} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' %}.
| {% if frontend_bundling == 'vite' %} | |
| {% if frontend_bundling is defined and frontend_bundling == 'vite' %} |
template/Dockerfile.jinja
Outdated
| # Copy rest of application | ||
| COPY . . | ||
|
|
||
| {% if frontend_bundling == 'vite' -%} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' -%}.
template/Dockerfile.jinja
Outdated
| # Copy rest of application | ||
| COPY . . | ||
|
|
||
| {% if frontend_bundling == 'vite' -%} |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
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' -%}.
- 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
There was a problem hiding this 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.
template/docker-compose.yml.jinja
Outdated
| - ./static:/app/static | ||
| ports: | ||
| - "5173:5173" | ||
| command: sh -c "npm install && npm run dev" |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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).
| command: sh -c "npm install && npm run dev" | |
| command: sh -c "npm ci && npm run dev" |
template/.gitignore.jinja
Outdated
|
|
||
| # Node | ||
| node_modules/ | ||
| frontend/node_modules/ |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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/| frontend/node_modules/ |
| 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() | ||
|
|
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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- 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
There was a problem hiding this 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") |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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.
template/Dockerfile.jinja
Outdated
| COPY frontend/ ./ | ||
|
|
||
| # Create output directory and build assets | ||
| RUN mkdir -p /app/static && npm run build |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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.
| RUN mkdir -p /app/static && npm run build | |
| RUN npm run build |
docs/features/frontend-options.md
Outdated
|
|
||
| | Issue | Solution | | ||
| |-------|----------| | ||
| | Vite HMR not working in Docker | Ensure `VITE_DEV_SERVER_HOST=vite` is set for the web service | |
Copilot
AI
Nov 30, 2025
There was a problem hiding this comment.
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'.
| | 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 | |
- Explicitly test Vite in all-features test - Remove redundant mkdir in Dockerfile (Vite creates dirs) - Clarify troubleshooting doc for VITE_DEV_SERVER_HOST
There was a problem hiding this 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.
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
What Changed
When users select HTMX+Tailwind frontend, they now get a follow-up question to choose between:
Template Changes
Components Affected
Testing Checklist
pytest)just testin generated project)Test Coverage
Breaking Changes
Migration Guide:
Backward Compatibility
copier updatewith user actionSecurity Considerations
Removes reliance on external CDNs (cdn.tailwindcss.com, unpkg.com, jsdelivr.net) for production deployments when Vite option is selected.
Documentation
Deployment Impact
Screenshots / Logs
Checklist
Additional Notes
Reviewer Notes: