Skip to content
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

Introduce vite to transpile js/css assets for development and production + vitest for static file testing #22957

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Dec 30, 2024

Relates to: mozilla/addons#2000

Description

Note

Don't be scared, most of the changes are package-lock.json

  • Introduce vite and vitest targeting only the preload.js and analytics.js scripts initially.
  • add infrastructure to support building and serving vite files

Context

This PR introduces the "plumbing" needed for compiling assets with vite. It does not migrate all assets to using vite. We should split this up after landing this and we can move assets over to esm with vite.

Vite

Vite will now compile every file in ./static/js/*.js and ./static/cs/*.less into independent bundles. Those files can then be included in our django app via the helper method vite_asset.

For example:

{{ vite_asset('js/preload.js') }}
{{ vite_asset('css/common.less') }}

This will render roughly to this:

<script src="<vite_asset_url>" />
<link href="<vite_asset_url" />

In development mode, vite_asset_url will resolve to /static/bundle/<path> where path is the original path to the url.
Nginx routes /static/bundle/ to the vite dev server which will then return the asset compiled in real time. LESS will return CSS and JS will return transpiled js.

In production mode, vite will compile the assets and save them in ./site-static/assets/ and a manifest will be used to resolve the url to the file path. Nginx will mount this path and serve files directly. In production environments, like before we will serve files from a CDN.

Testing

Dev mode

Run the dev/prod version of the app and make sure that:

  • assets are being served successfully
  • no console errors or runtime errors in the modified javascript
  • all functionality remains unregressed.
make up DOCKER_TARGET=development

Prod mode

make up DOCKER_TARGET=production

Automated tests

Run the following tests which are now using vitest instead of jest

  • make test_setup
  • make run_js_tests

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind force-pushed the vite-assets branch 15 times, most recently from 6ec5e0b to a33c53e Compare January 6, 2025 14:53
@KevinMind KevinMind force-pushed the vite-assets branch 2 times, most recently from de7b184 to ab67362 Compare January 7, 2025 13:40
@KevinMind KevinMind force-pushed the vite-assets branch 10 times, most recently from ec121bb to 2f87f8d Compare January 9, 2025 00:00
@KevinMind KevinMind requested review from a team and removed request for a team January 9, 2025 00:13
@KevinMind KevinMind requested a review from eviljeff January 9, 2025 00:13
@KevinMind KevinMind marked this pull request as ready for review January 9, 2025 00:13
@KevinMind KevinMind changed the title Vite-assets Introduce vite to transpile js/css assets for development and production + vitest for static file testing Jan 9, 2025
@KevinMind KevinMind force-pushed the vite-assets branch 8 times, most recently from 433e8fc to 75d2dc4 Compare January 28, 2025 15:59
@KevinMind KevinMind requested review from diox and removed request for eviljeff January 28, 2025 16:33
src/olympia/amo/templatetags/jinja_helpers.py Outdated Show resolved Hide resolved
STATIC_BUILD_PATH = path('static-build')
# This value should be kept in sync with vite.config.js
# where the manifest will be written to
STATIC_BUILD_MANIFEST_PATH = path(STATIC_BUILD_PATH, 'manifest.json')
Copy link
Member

Choose a reason for hiding this comment

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

A bit unfortunate that vite calls this manifest.json, same as webextensions manifest, but at least it doesn't conflict with django's staticfiles.json ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can make the file whatever we want. I could make it "vite-manifest.json" or "banana.json" as long as the file names are aligned in both places it's up to us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KevinMind KevinMind merged commit 1b0ff3a into master Jan 31, 2025
41 checks passed
@KevinMind KevinMind deleted the vite-assets branch January 31, 2025 17:20
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