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

server-side template rendering stack #389

Merged
merged 13 commits into from
Feb 1, 2024
Merged

server-side template rendering stack #389

merged 13 commits into from
Feb 1, 2024

Conversation

jondot
Copy link
Contributor

@jondot jondot commented Jan 24, 2024

Based off #385

Started design principles:

  • new first-class folder: assets, where any runtime asset go into, except configuration. this means we have a tally of 3 objects to deploy with server-side template rendering.
  1. main binary
  2. config dir
  3. assets dir
  • templates go in assets/templates
  • i18n go in assets/i18n, shared ftl is called shared.ftl to more simply indicate what it is (instead of core.ftl)
  • name of the function is t rather than fluent avoiding using implementation-specific things unless must (so we can swap out components)

Open tech issues:

  • setting locale from request headers
  • testing
  • right now implemented as an initializer
  • default context -- is it important?
  • how do we extend functions generically? i imagine users will say: i want my pluralization library, etc., what is their hook point to add stuff? (currently: modifying the initializer)
  • need to go over all unwraps, expects, and remove them when possible
  • tera should offer live-reload. will it work?, if so enable it only on development (or via config?)

Open design questions

  • (easy) how would configuration look like? we want strong opinions that would work, and leave only the minimum required to be configured (because we can always "open shop for configuration" later)
  • (medium) which of all objects do we generate via loco generate (probably: views, but maybe initializer and wiring into app hooks?)
  • (easy) if i18n does not exist physically, do we detect it to be missing and just not configure? should we give the user a notice? or should this be dependent on configuration?
  • (easy) what dependencies do we let the user take? what deps do we bring in as part of Loco (so the user does not need to edit their cargo config)
  • (medium) how does the user opt-in into this render stack? do we include it as default and allow opt-out? do we include it always without any opt-out?
  • (medium) Should we ship this as an Initializer?
  • (medium) does this current design allow for switching template engine to another easily enough? (not magically, just easily... I don't think people really need transparent template swapping in runtime)
  • (medium) how does this holistically combine with the static assets feature?

conclusions

rendering ergonomics

(psuedo code)

// controller.rs
render_home(e: Engine<TeraView>) -> Result<impl IntoResponse> {
   views::dashboard::home(e, current_user)
}

// src/views/dashboard.rs
home(e: eng, user: User) -> impl ...{
        format::template(e, "dashboard/home.html", json!(name: user.name))
}

Notes:

  1. adding a new view requires (1) a new template (2) a codification of domain-to-template data munging and separation of concern hopefully alleviated by a generator
  2. controller is "talking domain" which is a Good Thing
  3. "src/views" remains a clear concept and we're separating the concept of a template = a mechanical thing, from a view = a live, rich, Loco framework concept.
  4. using render becomes a pit of success. by typing render:: you get a list of all the things you can do which is a Good Thing. Adding a "render type" is giving the user more cognitive load and only should be a secondary thing.

view generator

yes. creating files in the correct opinionated structure is what's expected from the framework: "where you have strong opinions, have a generator supporting it". generate view accounts/dashboard --i18n (topic/page). the i18n flag will create the i18n base structure if missing (folders, shared, etc.), and then add a en-US/ ??? (need to see if we can follow a accounts/dashboard.ftl structure, or just accounts_dashboard.ftl. **this also means we're calling the "thing" view.

editor notes: do we want generate template instead? both view and templates are overloaded. Also, if we go with generate view this means its better the whole feature must be part of Loco with no option to remove it. I think we already "payed" for tera so no harm done there (we'll pay extra for fluent-templates, but maybe thats OK)

@jondot jondot added this to the 0.3.0 milestone Jan 24, 2024
@jondot
Copy link
Contributor Author

jondot commented Jan 25, 2024

@alexhumphreys happy to get your opinion on any of the design issues or other checkbox items above

@alexhumphreys
Copy link

@jondot I'm sure you know more than me on this, but I'll give my opinions on the few where I have one:

Open tech issues:

setting locale from request headers

I guess the solution will eventually involve fromRequestParts, so seems sensible to grab the header there, as I tried here.

testing

Tried to add a few here, wouldn't say they were good, but could be a place to start.

default context -- is it important?

Good question! I genuinely don't know, I kinda just took it from the example in the fluent-template README.

Open design questions:

(easy) how would configuration look like? we want strong opinions that would work, and leave only the minimum required to be configured (because we can always "open shop for configuration" later)

Agreed. I guess a lot of things that need to be passed are paths, if there's standard locations for these in the assets folder you could get away with defaults and only need to pass values when one is changing from the default paths.

(medium) which of all objects do we generate via loco generate (probably: views, but maybe initializer and wiring into app hooks?)

Good question. I can see arguments for either nothing, or for a minimal example that's easy to extend.

(easy) if i18n does not exist physically, do we detect it to be missing and just not configure? should we give the user a notice? or should this be dependent on configuration?

Not sure what you mean by "if i18n does not exist physically"

(medium) how does the user opt-in into this render stack? do we include it as default and allow opt-out? do we include it always without any opt-out?

@slyons mentioned feature gating here, which seems like a good idea. No clue on opt-in vs opt-out, maybe it depends what kind of app you're generating?

(medium) does this current design allow for switching template engine to another easily enough? (not magically, just easily... I don't think people really need transparent template swapping in runtime)

Agree. I guess one can't have fully transparent template swapping anyway, the templates themselves probably have different syntax and need different args.

@jondot
Copy link
Contributor Author

jondot commented Jan 30, 2024

@alexhumphreys rendering is now feature complete with API that should work for all scenarios (including building a custom view engine). happy to take comments relating to ergonomics, usability and ideas that we can add.

Some more decisions that happened:

  • Since Tera is a dep we already take, I decided to have a builtin Tera view engine, vanilla
  • The initializer has the role to add i18n, and customizations if needed to this Tera engine
  • At any point in time you can build a "competing" view engine, custom view engine of your own, and you can use as many of those as you want -- as long as you can take the deps, and configure an initializer per engine (in the example you have a Hello view engine as a demo)

luck has it, that from what I see we need no configuration at all.

@jondot jondot merged commit f6fae06 into master Feb 1, 2024
13 checks passed
@jondot jondot changed the title draft: template rendering stack server-side template rendering stack Feb 1, 2024
@alexhumphreys
Copy link

Nice one, thanks for all the work on this! Testing now on examples/demo and it works, sweet!

If I come up with a nice way to deal with extracting the locale header and passing it to the lang var arg in the templates, I'll open a PR with that 👍

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