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

Prefix URLs with custom origin to serve assets from CDN #105

Open
mcferden opened this issue Nov 25, 2023 · 9 comments
Open

Prefix URLs with custom origin to serve assets from CDN #105

mcferden opened this issue Nov 25, 2023 · 9 comments

Comments

@mcferden
Copy link

In my production build I'd like to add CDN prefix to URLs generated by vite_asset, so that /static/assets/app-[hash].js will become https://cdn.example.com/static/assets/app-[hash].js.

I tried to play with static_url_prefix config variable, but it seems to break manifest resolution.

Is there some way to do this properly?

@jacobb
Copy link

jacobb commented Nov 25, 2023

vite_asset should respect STATIC_URL in this regard

@Niicck
Copy link
Collaborator

Niicck commented Nov 25, 2023

During dev mode, django-vite will respect STATIC_URL. But in the case of a production build, django-vite is going to look to your STATIC_ROOT instead.

In your settings you should set STATIC_ROOT=https://cdn.example.com/static/.

The default formula for discovering your manifest.json is this:

STATIC_ROOT / static_url_prefix / "manifest.json".

And it looks like your vite assets live right under the /static/ directory, so I'm guessing that's where your manifest.json lives too. I'm also guessing there isn't a need for you to set a static_url_prefix in this case, just a STATIC_ROOT.

@mcferden
Copy link
Author

As Django docs say:

STATIC_ROOT
The absolute path to the directory where collectstatic will collect static files for deployment.

The way my deployment works is it collects static files locally and then they are served by Nginx acting like CDN upstream. So I don't use custom STATICFILES_STORAGE to push static file to CDN.

Changing STATIC_ROOT to https://cdn.example.com/static/ breaks collectstatic, and django-vite can't find manifest.json, because it's not on CDN. Also, I don't need this prefix on all my static files, just on Vite assets.

@Niicck
Copy link
Collaborator

Niicck commented Nov 26, 2023

Whoops. My bad, @jacobb was right. Yeah STATIC_ROOT is only used for collectstatic, I think you're going to want to set your STATIC_URL = https://cdn.example.com/static/. From there, adjust your static_url_prefix (if necessary) to point to the location of your manifest.json within that /static/ directory.

What is your STATIC_URL currently set to? Does your nginx setup work for other {% static %} django assets, and only doesn't work with assets from django-vite?

@mcferden
Copy link
Author

mcferden commented Nov 26, 2023

Currently STATIC_URL is set to /static/. The problem with changing it is that all static files will be prefixed with CDN origin.

My current setup now is simple: custom tag on_cdn adds prefix based on environment, so for some files in template I can write <script src="{% on_cdn %}{% static app.js' %}"></script>.

One way I see to achieve the same behavior is to make another custom tag cdn_vite_asset similar to vite_asset and postprocess generated tags there.

@jacobb
Copy link

jacobb commented Nov 26, 2023

a new template tag is likely over-complicating it, depending on how you determine what environment you're in.

if ENVIRONMENT == "prod":
    STATIC_URL = "https://cdn.example.com/static/"
else:
    STATIC_URL = "/static/"
    # other handy dev static-related settings, such as DJANGO_VITE_DEV_MODE

Is there a situation where you would want to serve some files from the CDN, but not everything? If so, a template tag that forces a particular STATIC_URL may be called for, but depending on the use case something else may be best.

@caffodian
Copy link

caffodian commented Mar 22, 2024

Joining the bandwagon here - we also have this issue due to transitioning between old(legacy) UI and new vite-based UI. Our vite stuff lives on a different CDN for various reasons.

It might be worth having a setting that can take a callable or something and resolves the base url accordingly (the default implementation will just return STATIC_URL as it is now.).

I'll see if there's any clever ways around this for now. I think it might be a bit more involved due to staticfiles_storage.url being used under the hood....if anyone has found any clever workarounds to this, please let me know. :)

EDIT: I ended up doing a hacky monkeypatch, something like this in the settings:

    def monkeypatched_get_production_server_url(self, path: str) -> str:
        # same
        production_server_url = path
        if prefix := self.static_url_prefix:
            if not prefix.endswith("/"):
                prefix += "/"
            production_server_url = urljoin(prefix, path)

        # THIS IS WHERE THE SHENANIGANS BEGIN
        from django.conf import settings

        if hasattr(settings, "SPECIAL_CDN_URL"):
            production_server_url = urljoin(settings.SPECIAL_CDN_URL, path)

        return production_server_url

    from django_vite.core.asset_loader import DjangoViteAppClient

    DjangoViteAppClient._get_production_server_url = (
        monkeypatched_get_production_server_url
    )

And it works for me for now. Just sharing in case someone else finds this thread, since I don't think changing the staticfiles behaviour is wise for the default case, vs. introducing this to a much older commercial project.

@Niicck
Copy link
Collaborator

Niicck commented Mar 24, 2024

Thanks for all the feedback. I'm going to take another look at providing better support for CDN assets. I'll post back when I get an alpha release up for people to test.

@Niicck
Copy link
Collaborator

Niicck commented Jun 21, 2024

Here's one option: #138

Feel free to share any thoughts/suggestions.

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

No branches or pull requests

4 participants