-
Notifications
You must be signed in to change notification settings - Fork 75
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
Enable multi-app configs #92
Conversation
…t and _load_css_files_of_asset
Hi, when this PR could be merged? I really need that functionality. |
I can merge it. Should we release it with a major version bump, to 3.0? |
Yeah, I think a bump to 3.0 would be good. And while we're at it, I think the release to 3.0 would be a good time to change the default port to 5173. #51 |
Are you currently available to apply that change? |
This PR adds the ability to use multiple vite configs.
Check out this repo for examples of how to use it: https://github.com/Niicck/django-vite-examples
Previous discussions:
What does this PR change?
DJANGO_VITE={...}
settingsDJANGO_VITE_ASSETS_PATH
1. There are new settings
Instead of tying django-vite config values to root settings values (
DJANGO_VITE_DEV_SERVER_HOST
,DJANGO_VITE_DEV_SERVER_PORT
, ...) we can now set values for distinct apps within a singleDJANGO_VITE
setting. This matches the way Django's database and cache backends are set, as well as django-webpack-loader.In
settings.py
:In your templates, you can specify the app you want to use:
If you don't specify an app, it'll use the "default" config.
This is totally backwards compatible with our existing settings. For now, we still parse the legacy django-vite settings and assign them to the "default" app. Developers can keep using django-vite as though nothing has changed.
2. DjangoViteAssetLoader has been split up
MrBin started an implementation of this feature last year (#50). I started by bringing that implementation up to date with master (Niicck#1). While doing that I found some opportunities to split our logic up into discrete classes.
Splitting up responsibilities made it a lot easier to safely isolate configs between apps. It should also make future testing and development easier.
The core of the django-vite logic has been split into these 4 classes:
settings.py
and constructs DjangoViteAppClients for each app that we define. But now it routes methods likegenerate_vite_asset
to DjangoViteAppClient.3. DJANGO_VITE_ASSETS_PATH is removed
That's the one setting I didn't port over. Turns out, it was required, but never actually used.
The only purpose of
DJANGO_VITE_ASSETS_PATH
is to create theDJANGO_VITE_STATIC_ROOT
when we're in dev_mode. ButDJANGO_VITE_STATIC_ROOT
is never used when we're in dev_mode.The docs are correct, wherever you do end up placing your compiled vite assets, that should be added to
STATICFILES_DIRS
. But that's true of any static file in Django. There's nothing special that necessitates the use of this particular variable. Definitely keep the note about putting your compiled assets into STATICFILES_DIRS, but drop the requirement to set that explicit variable.What's going to change for people who don't want to use multiple vite apps?
If they want to keep using the legacy settings, they are welcome to do so. This PR is 100% backwards compatible. But existing users would not longer need to set
DJANGO_VITE_ASSETS_PATH
.Next steps
We're ready to launch!
I found 1 bug with doing HMR with multiple apps on the same page, but it's not a regression so I'm not worried about it right now.
It'd be good to get some eyes on the code. Feel free to share your thoughts, ideas, concerns.