-
-
Notifications
You must be signed in to change notification settings - Fork 18
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 Jinja support #199
base: main
Are you sure you want to change the base?
Add Jinja support #199
Conversation
54906cf
to
1384632
Compare
OK, this now render the "Hello World!" correctly. Feedback requested before thinking about tests/docs etc. |
1384632
to
2d282e4
Compare
2d282e4
to
7e97489
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments since I took a early peak at the draft.
1a8b8a7
to
d432639
Compare
Resolved last outstanding comment. And ran black. If we are to proceed, I'd propose to squash the current commits before thinking about tests/packaging/docs etc. (FWIW, in that regard, I assume the appropriate packaging would be as something like "reactpy_django[jinja]"...but I'm not currently sure how to go about implementing that). |
Optional dependencies can be defined within the package in setup.py. For example: {
"extras_require": {
"encryption": ["cryptography", "pycryptodome"],
},
...
} |
Thanks, I'm more or less familiar with this bit. I was more thinking about the new file I have added (and any supporting test file), and whether that could/should be omitted unless "[jinja] was specified? |
I have some changes in one of my PRs that will simplify test configuration TLDR: Our test suite can now run multiple different Django settings.py files. And yes, from the user's perspective we should have all the jinja dependencies be optional via |
My PR has been merged. All There shouldn't be anything blocking this PR anymore, so you'll need to do the following:
|
Noted. I've a full plate right now but have this on my radar. |
Description
WORK IN PROGRESS: DO NOT MERGE.
This is incomplete support for Jinja2-based templates. I could use some pointers to finish the missing part (see comments at the end of src/reactpy_django/templatetags/jinja.py).(also missing, docs, packaging and tests)
To reproduce the results to date requires the following example changes on the Django side...
Configure template files ending in ".jinja" to be processed via Jinja:
Add the new app:
File of components in
components.py
:Test view in
views/react.py
:Template for the view
templates/my-template.html.jinja
:project/asgi.py
And finally
project/urls.py
:Once Django is restarted, navigating to the view, you should see
Hello World!
Checklist:
Please update this checklist as you complete each item:
By submitting this pull request you agree that all contributions comply with this project's open source license(s).