Skip to content

Migrate remaining files to vite #23180

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

Merged
merged 2 commits into from
Mar 20, 2025
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
24 changes: 9 additions & 15 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"less": "4.2.2",
"netmask": "2.0.2",
"source-map": "0.7.4",
"timeago": "1.6.7",
"timeago.js": "^4.0.2",
"underscore": "1.13.7"
},
"devDependencies": {
Expand Down
5 changes: 1 addition & 4 deletions src/olympia/amo/management/commands/compress_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ def handle(self, **options):
# Concat all the files.
tmp_concatted = '%s.tmp' % concatted_file
if len(files_all) == 0:
raise CommandError(
'No input files specified in '
'MINIFY_BUNDLES["%s"]["%s"] in settings.py!' % (ftype, name)
)
return
with open(tmp_concatted, 'w') as f:
f.write(''.join(contents))

Expand Down
48 changes: 8 additions & 40 deletions src/olympia/core/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
import os
import subprocess
import warnings
from io import StringIO
from pathlib import Path
from pwd import getpwnam

from django.apps import AppConfig
from django.conf import settings
from django.core.checks import Error, Tags, register
from django.core.management import call_command
from django.core.management.base import CommandError
from django.db import connection
from django.utils.translation import gettext_lazy as _

Expand Down Expand Up @@ -87,68 +85,38 @@ def version_check(app_configs, **kwargs):
@register(CustomTags.custom_setup)
def static_check(app_configs, **kwargs):
errors = []
output = StringIO()
version = get_version_json()

# We only run this check in production images.
if version.get('target') != 'production':
return []

try:
call_command('compress_assets', dry_run=True, stdout=output)
stripped_output = output.getvalue().strip()

if stripped_output:
file_paths = stripped_output.split('\n')
for file_path in file_paths:
if not os.path.exists(file_path):
error = f'Compressed asset file does not exist: {file_path}'
errors.append(
Error(
error,
id='setup.E003',
)
)
else:
errors.append(
Error(
'No compressed asset files were found.',
id='setup.E003',
)
)

except CommandError as e:
errors.append(
Error(
f'Error running compress_assets command: {str(e)}',
id='setup.E004',
)
)
manifest_path = Path(settings.STATIC_BUILD_MANIFEST_PATH)

if not os.path.exists(settings.STATIC_BUILD_MANIFEST_PATH):
if not manifest_path.exists():
errors.append(
Error(
(
'Static build manifest file '
f'does not exist: {settings.STATIC_BUILD_MANIFEST_PATH}'
f'does not exist: {manifest_path.as_posix()}'
),
id='setup.E003',
)
)
else:
with open(settings.STATIC_BUILD_MANIFEST_PATH, 'r') as f:
with open(manifest_path.as_posix(), 'r') as f:
manifest = json.load(f)

for name, asset in manifest.items():
# Assets compiled by vite are in the static root directory
# after running collectstatic. So we should look there.
path = os.path.join(settings.STATIC_ROOT, asset['file'])
if not os.path.exists(path):
path = Path(settings.STATIC_ROOT) / asset['file']
if not path.exists():
errors.append(
Error(
(
f'Static asset {name} does not exist at '
f'expected path: {path}'
f'expected path: {path.as_posix()}'
),
id='setup.E003',
)
Expand Down
132 changes: 77 additions & 55 deletions src/olympia/core/tests/test_apps.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import json
import os
import tempfile
from unittest import mock

from django.core.management import CommandError, call_command
from django.core.management import call_command
from django.core.management.base import SystemCheckError
from django.test import TestCase
from django.test.utils import override_settings
Expand Down Expand Up @@ -34,14 +35,11 @@ def setUp(self):
with open(self.fake_css_file, 'w') as f:
f.write('body { background: red; }')

patch_command = mock.patch('olympia.core.apps.call_command')
self.mock_call_command = patch_command.start()
self.mock_call_command.side_effect = (
lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n')
)
self.addCleanup(patch_command.stop)

self.media_root = tempfile.mkdtemp(prefix='media-root')
self.static_root = tempfile.mkdtemp(prefix='static-root')
self.manifest_path = os.path.join(
tempfile.mkdtemp(prefix='manifest-dir'), 'manifest.json'
)

@mock.patch('olympia.core.apps.connection.cursor')
def test_db_charset_check(self, mock_cursor):
Expand Down Expand Up @@ -104,63 +102,87 @@ def test_illegal_override_uid_check(self, mock_getpwnam):
with override_settings(HOST_UID=1000):
call_command('check')

def test_static_check_no_assets_found(self):
"""
Test static_check fails if compress_assets reports no files.
"""
self.mock_get_version_json.return_value['target'] = 'production'
# Simulate "compress_assets" returning no file paths.
self.mock_call_command.side_effect = (
lambda command, dry_run, stdout: stdout.write('')
)
@override_settings(STATIC_BUILD_MANIFEST_PATH='/nonexistent/path/manifest.json')
def test_static_check_manifest_does_not_exist(self):
"""Test that an error is raised when the manifest file doesn't exist."""
with self.assertRaisesMessage(
SystemCheckError, 'No compressed asset files were found.'
SystemCheckError,
'Static build manifest file does not exist: '
'/nonexistent/path/manifest.json',
):
call_command('check')

@mock.patch('os.path.exists')
def test_static_check_missing_assets(self, mock_exists):
"""
Test static_check fails if at least one specified compressed
asset file does not exist.
"""
self.mock_get_version_json.return_value['target'] = 'production'
# Simulate "compress_assets" returning a couple of files.
self.mock_call_command.side_effect = (
lambda command, dry_run, stdout: stdout.write(
f'{self.fake_css_file}\nfoo.js\n'
)
)
# Pretend neither file exists on disk.
mock_exists.return_value = False
@override_settings(STATIC_ROOT='/static/root', STATIC_BUILD_MANIFEST_PATH=None)
def test_static_check_manifest_references_nonexistent_files(self):
"""Test that an error is raised when manifest references non-existent files."""
# Create a temporary manifest file with references to non-existent files
with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
manifest_content = {
'app.js': {'file': 'app.123abc.js'},
'styles.css': {'file': 'styles.456def.css'},
}
json.dump(manifest_content, f)
manifest_path = f.name

with override_settings(STATIC_BUILD_MANIFEST_PATH=manifest_path):
with self.assertRaisesMessage(
SystemCheckError,
'Static asset app.js does not exist at expected path: '
'/static/root/app.123abc.js',
):
call_command('check')

with self.assertRaisesMessage(
SystemCheckError,
# Only the first missing file triggers the AssertionError message check
'Compressed asset file does not exist: foo.js',
):
os.unlink(manifest_path)

@override_settings(STATIC_BUILD_MANIFEST_PATH=None)
def test_static_check_empty_manifest(self):
"""Test that no error is raised with an empty manifest."""
# Create an empty manifest file
with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
json.dump({}, f)
manifest_path = f.name

with override_settings(STATIC_BUILD_MANIFEST_PATH=manifest_path):
# No error should be raised with an empty manifest
call_command('check')

def test_static_check_command_error(self):
"""
Test static_check fails if there's an error during compress_assets.
"""
self.mock_get_version_json.return_value['target'] = 'production'
self.mock_call_command.side_effect = CommandError('Oops')
with self.assertRaisesMessage(
SystemCheckError, 'Error running compress_assets command: Oops'
os.unlink(manifest_path)

def test_static_check_valid_manifest(self):
"""Test that no error is raised when manifest references existing files."""
# Create a temporary static root with actual files
static_root = tempfile.mkdtemp(prefix='static-root-valid')

# Create the asset files
asset1_path = os.path.join(static_root, 'app.123abc.js')
asset2_path = os.path.join(static_root, 'styles.456def.css')

with open(asset1_path, 'w') as f:
f.write('console.log("test");')

with open(asset2_path, 'w') as f:
f.write('body { color: blue; }')

# Create a manifest file that references these files
with tempfile.NamedTemporaryFile(mode='w', delete=False) as f:
manifest_content = {
'app.js': {'file': 'app.123abc.js'},
'styles.css': {'file': 'styles.456def.css'},
}
json.dump(manifest_content, f)
manifest_path = f.name

with override_settings(
STATIC_ROOT=static_root, STATIC_BUILD_MANIFEST_PATH=manifest_path
):
# No error should be raised with a valid manifest
call_command('check')

def test_static_check_command_success(self):
"""
Test static_check succeeds if compress_assets runs without errors.
"""
self.mock_get_version_json.return_value['target'] = 'production'
self.mock_call_command.side_effect = (
lambda command, dry_run, stdout: stdout.write(f'{self.fake_css_file}\n')
)
call_command('check')
# Clean up
os.unlink(manifest_path)
os.unlink(asset1_path)
os.unlink(asset2_path)
os.rmdir(static_root)

def test_nginx_skips_check_on_production_target(self):
fake_media_root = '/fake/not/real'
Expand Down
4 changes: 2 additions & 2 deletions src/olympia/devhub/templates/devhub/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
{% block title %}{{ dev_page_title() }}{% endblock %}

{% block extrahead %}
{{ css('zamboni/devhub') }}
{{ vite_asset('css/devhub.less') }}
{% endblock %}

{% block site_header_title %}
{% include "devhub/nav.html" %}
{% endblock %}

{% block js %}
{{ js('zamboni/devhub') }}
{{ vite_asset('js/devhub.js') }}
{% endblock %}

{% block footer_extras %}
Expand Down
13 changes: 4 additions & 9 deletions src/olympia/devhub/templates/devhub/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
href="{{ url('amo.opensearch') }}" />

{% block site_css %}
{{ css('common/fonts') }}
{{ css('devhub/new-landing/css') }}
{{ css('common/footer') }}
{{ vite_asset('css/fonts.less') }}
{{ vite_asset('css/devhub-new-landing.less') }}
{{ vite_asset('css/footer.less') }}
{% endblock %}

{% block extrahead %}{% endblock %}
Expand All @@ -36,13 +36,8 @@

{% if settings.DEV_MODE %}
{{ vite_hmr_client() }}
{% if settings.LESS_LIVE_REFRESH %}
<meta id="live_refresh" name="live_refresh" value="1">
{% endif %}
{{ js('debug') }}
{% endif %}

{{ js('jquery_base') }}
{{ vite_asset('js/preload.js') }}
</head>
{% set user_authenticated = request.user.is_authenticated %}
Expand Down Expand Up @@ -82,7 +77,7 @@
</div>
{% block site_js %}
<script src="{{ static('js/i18n/%s.js'|format(LANG)) }}"></script>
{{ js('devhub/new-landing/js') }}
{{ vite_asset('js/devhub-new-landing.js') }}
{% endblock %}
</body>
</html>
Loading
Loading