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

Updates to vue3 templatetag for supporting Vite build #88

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

bbonf
Copy link
Contributor

@bbonf bbonf commented Oct 6, 2023

@bbonf bbonf requested a review from tymees October 6, 2023 07:46
@bbonf bbonf changed the base branch from master to develop October 6, 2023 07:46
Copy link
Member

@tymees tymees left a comment

Choose a reason for hiding this comment

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

I like the idea, I was missing something like this and was planning on writing it myself.

However, I would like it to be a bit more modular/configurable

Comment on lines +139 to +141
# read manifest json (based on vite output)
with open(settings.VUE_MANIFEST) as f:
manifest = json.load(f)
Copy link
Member

Choose a reason for hiding this comment

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

This only makes sense if you only have the one Vue app for the entire app; this will clash with the use-cases that DSC itself will encounter; (Multiple components, only some of which are useful for any given app).

Ideally, you can specify manifests in a more configurable way; in the Vue 2 app I build a 'app registry', which is just a simple class with info like this, grouped together under a single identifier one can use to load all you need. Something like that would be nicer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does vite do in that use case? Separate modules per component with separate manifests?

Copy link
Member

Choose a reason for hiding this comment

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

In that case, there would be seperate (semi-)independent vite builds, so you would get separate manifests per 'app'.

For example, you get separate manifests for:

  • The default List-view
  • Any custom List-view
  • Vue-powered form-widgets

Comment on lines +144 to +146
static = settings.STATIC_URL + 'vue/'
if hasattr(settings, 'VUE_URL'):
static = settings.VUE_URL
Copy link
Member

Choose a reason for hiding this comment

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

Why not use Django's built-in static resolving instead?

@tymees
Copy link
Member

tymees commented Oct 6, 2023

(To explain my registry thing a bit more)

In this case would just be a simple singleton class, that is populated by adding a key-value to it in the app.py of individual Django apps. See also:
https://github.com/DH-IT-Portal-Development/django-shared-core/blob/master/src/cdh/vue/components.py

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