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

Add dev mode only tags #83

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

emab
Copy link
Contributor

@emab emab commented Jun 5, 2023

Address #62

I'm not sure if this approach is too simple - but as far as I can tell this should achieve what is needed. Doesn't add any overhead if you're not using the tags.

@thijskramer
Copy link
Collaborator

Thanks for your contributions, @emab!

Just a thought: wouldn't it be cleaner to add an extra parameter (i.e. dev_only=True) to the already existing {% vite_asset %} and {% vite_asset_url %} templatetags?

Comment on lines 657 to 682
@register.simple_tag
@mark_safe
def vite_dev_asset_url(
path: str,
) -> str:
"""
Generates only the URL of an asset managed by ViteJS if
DJANGO_VITE_DEV_MODE is True.
Warning, this function does not generate URLs for dependant assets.

Arguments:
path {str} -- Path to a Vite asset.

Raises:
RuntimeError: If cannot find the asset path in the
manifest (only in production).

Returns:
str -- The URL of this asset.
"""
assert path is not None

if not DJANGO_VITE_DEV_MODE:
return ""

return DjangoViteAssetLoader.instance().generate_vite_asset_url(path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly sure if this one brings any value - how would it be used? 🤔

@emab
Copy link
Contributor Author

emab commented Jun 6, 2023

Thanks for your contributions, @emab!

Just a thought: wouldn't it be cleaner to add an extra parameter (i.e. dev_only=True) to the already existing {% vite_asset %} and {% vite_asset_url %} templatetags?

That's probably not a bad approach either actually, happy to implement that way if you'd prefer?

To clarify - how would we expect vite_asset_url to be used in for dev-only scenarios? I'd assume it would be used like:

<link rel="stylesheet" href="{% vite_asset_url 'path/to/my/asset.css' dev_only=True %}" />

So I'm a bit unclear if having it return nothing is suitable - it makes much more sense for the vite_asset tag though.

@thijskramer
Copy link
Collaborator

Thanks for your contributions, @emab!
Just a thought: wouldn't it be cleaner to add an extra parameter (i.e. dev_only=True) to the already existing {% vite_asset %} and {% vite_asset_url %} templatetags?

That's probably not a bad approach either actually, happy to implement that way if you'd prefer?

Yes please, that would make the code a bit more DRY I think.

To clarify - how would we expect vite_asset_url to be used in for dev-only scenarios? I'd assume it would be used like:

<link rel="stylesheet" href="{% vite_asset_url 'path/to/my/asset.css' dev_only=True %}" />

So I'm a bit unclear if having it return nothing is suitable - it makes much more sense for the vite_asset tag though.

I'm not sure, since I've discovered this package I've never used vite_asset_url so I'm unaware of the added value and use cases for it...

@emab
Copy link
Contributor Author

emab commented Jun 8, 2023

I'm not sure, since I've discovered this package I've never used vite_asset_url so I'm unaware of the added value and use cases for it...

In my setup, some of my entry points in Vite are CSS files - so I use the vite_asset_url to place them in my application, e.g as above:

<link rel="stylesheet" href="{% vite_asset_url 'path/to/my/asset.css' dev_only=True %}" />

Which seems to work pretty well! However - if vite_asset_url returned nothing in dev mode I don't think it would make sense.

I'll update this to allow dev_only kwarg on the vite_asset tag as I think it makes more sense there.

@emab
Copy link
Contributor Author

emab commented Jun 8, 2023

@thijskramer I've pushed an update which I've tested locally.

@Niicck
Copy link
Collaborator

Niicck commented Nov 15, 2023

Instead of:

{% vite_asset '<path to your asset>' dev_only=True %}

Could we just do:

{% if DJANGO_VITE_DEV_MODE %}
    {% vite_asset '<path to your asset>' %}
{% endif %}

It seems like this functionality could be handled on the project-level without needing to add anything to django-vite. For comparison, it doesn't seem like similar projects like django-webpack-loader or vite-ruby have support for this.

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.

3 participants