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

Move away from using the body var for Jinja2 templates #80

Merged
merged 3 commits into from
Aug 7, 2021
Merged

Move away from using the body var for Jinja2 templates #80

merged 3 commits into from
Aug 7, 2021

Conversation

floodpants
Copy link
Contributor

Move away from template.render(body=foobar) to a more dynamic approach. make_dict() is there to simplify validation of various source data types (eg creating a dict from a list of tuples etc). This collectively should allow and encourage greater re-use of J2 templates throughout a given app.

BC Break: This will mean that any templates which currently use {{ body.foobar }} will need to switch to {{ foobar }}, which is a pretty noticeable BC break, however given fastapi-mail is currently v0.x I think that's likely acceptable. There are workarounds to attempt to provide both levels of functionality if a smoother transition is desired but I'm not sure they're fool proof.

@sabuhish
Copy link
Owner

sabuhish commented Aug 4, 2021

Thanks one more time for bringing such a brilant changes. I think we should mention it in our docs so people can work around it, otherwise, they will have troubles. If you have time to make changes go ahead if not let me know I will make changes in docs as well. Thanks!

@floodpants
Copy link
Contributor Author

Yep, that's a good plan. I'll make some changes to the docs over the weekend and do an updated PR

@floodpants
Copy link
Contributor Author

Updated the docs to expand on the Jinja2 section a bit and added a note for a "legacy" method in <= 0.4.0. Let me know what you think

@sabuhish
Copy link
Owner

sabuhish commented Aug 7, 2021

That's awesome! That's what I was thinking as well, thanks for the great work!

@sabuhish sabuhish merged commit 9c40c8a into sabuhish:master Aug 7, 2021
sabuhish pushed a commit that referenced this pull request Aug 7, 2021
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