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

Request for feedback on Template Modules beta #151

Closed
dillonkearns opened this issue Oct 28, 2020 · 10 comments
Closed

Request for feedback on Template Modules beta #151

dillonkearns opened this issue Oct 28, 2020 · 10 comments

Comments

@dillonkearns
Copy link
Owner

I need to go through a few more iterations before Template Modules are ready to go out of beta. I need feedback to get to that point, and I would love to hear any ideas in this thread!

Trying out the Template Modules beta

You can upgrade your current app to use the Template Modules beta using these instructions: https://github.com/dillonkearns/elm-pages/blob/master/docs/7.0.0-elm-package-upgrade-guide.md#3---beta-template-modules-feature.

Or you can try out the template-modules branch in the elm-pages-starter repo.

Note that there is a command to generate a simple scaffolded Template Module. You can run npx elm-pages-generate Recipe, for example, to build a scaffold at src/Template/Recipe.elm.

What kind of feedback do I need?

  • What problems does this solve?
  • What problems does this not solve that you would like it to?
  • What is confusing about the initial setup?
  • What is confusing about adding new Template Modules once you get started

I'd love to hear both what is working well and what could be improved. Thanks for any and all feedback!

@dillonkearns
Copy link
Owner Author

Here are some things I want to improve (feedback welcome on all these points).

Template Builder for Shared module

I'd like to have an API for building up Shared.template, similar to how you can incrementally add features in a Template Module. One of the tricky things I'm trying to figure out is how to extract the type alias for SharedTemplate without having circular imports, and also without too many type variables. Having that type alias defined in the Shared module won't work long-term because the user should be able to freely change code in this module, but they aren't allowed to change that type alias, which is confusing.

Better developer experience for the TemplateType and variants

Right now, users have to manually add a variant, with exactly 1 argument, for every module in src/Template.

One nice improvement might be to generate TemplateType.elm. Then the question becomes, how do I know which type to use for the arguments? Let's take the example of a Recipe.elm template.

  • We can't use a type exposed by src/Template/Recipe.elm, because importing Template.Recipe in TemplateType would cause a circular dependency
  • We could have the user define a module where they expose a type for each given Template (and the name must match). Let's say it's called src/TemplateTypes.elm. Then the generated TemplateType module would just import TemplateTypes and generate the variant with type TemplateType = Recipe TemplateTypes.Recipe | -- ....

Planned improvements

  • Wire in StaticPayload to more places (init, update, and in the Site module)
  • Don't require user to define/expose type alias Msg = Never and type Model = () if they're using a template with no state.

@dillonkearns dillonkearns pinned this issue Oct 28, 2020
@sevfurneaux
Copy link
Contributor

Great work! 💪 I really like the idea of the Template Modules, and how it can make things so much more streamlined.

One bug that I've found from initially playing around with the elm-pages-starter branch, is that if I remove both Template.Article and Template.BlogIndex, then I get the following error in TemplateModulesBeta.elm:

Compiling ...-- REDUNDANT PATTERN ------------------------------- gen/TemplateModulesBeta.elm

The 2nd pattern is redundant:

236|             case metadata of
237|                 M.Page templateMetadata ->
238|                     Template.Page.template.subscriptions
239|                         templateMetadata
240|                         path
241|                         templateModel
242|                         model.global
243|                         |> Sub.map MsgPage
244| 
245|>                _ ->
246|                     Sub.none

My assumption is that Sub.none should be programmatically removed if only one Template module exists?

What is confusing about the initial setup?
Initially, I was confused that I needed to edit both the Template modules and TemplateType.elm in order for TemplateModulesBeta.elm to be updated. I'm not even sure if this is technically possible (because of what you mentioned above about circular dependencies), but having just one place (the template modules) rather than two places that need to be edited for TemplateModulesBeta.elm to update would be great.

@dillonkearns
Copy link
Owner Author

Oh good catch on the wildcard _ clause, I missed that one. That should definitely be fixed, thanks for letting me know 🙏

Yeah, unfortunately I don't think it's possible to define the Metadata types directly in the Template Module and let that be the source of truth. But I'm thinking it would work pretty nicely to just have a single module where you define only the types, and then auto-generate the custom type variants based on that.

Something like

module TemplateData exposing (..)

import Time

type alias Article = { title : String, publishDate : Time.Posix }

type alias Page = { title : String, description : String }

-- ... other types here
-- one for each module in `src/Template`

Then I can automatically generate the custom type. I could even include it in the generated code in Template.elm (which is used to build up the template components).

module Template exposing (..)

import TemplateData

type Data
  = Article TemplateData.Article
  | Page TemplateData.Page

@sevfurneaux
Copy link
Contributor

This is super interesting! 🧐 Based on your example, I'm wondering if the TemplateData type aliases could be generated with a placeholder structure when npx elm-pages-generate [Template] is run.

For example, running npx elm-pages-generate Recipe would...

  1. Create Recipe.elm in the Template folder
  2. Create this placeholder type alias in TemplateData.elm:

type alias Recipe = { title : String }

You could then just edit this if you want.

Maybe there are also CLI flags for staticData and noStaticData too? 🤯

It's pretty interesting what you could do based on these elm-pages-generate commands! Kinda rails-y 🙃.

Maybe in the future there could be a npx elm-pages init command, similar to elm-spa init? One could then choose whether a project is generated with Html or elm-ui.

@dillonkearns
Copy link
Owner Author

Generating the custom type and variants is feeling like a pretty clear win, so I'm looking forward to building that soon and seeing how it feels.

I've thought about the generate command for different variations of builder, too. It's an interesting one, because with the builder pattern, it's set up to be able to easily add things in as needed. But that said, I think it's still useful to have both (that is, a builder pattern to easily transition, and generator helpers to easily initialize with different permutations).

My thinking on generating a starting point for the TemplateData type aliases is that I try to avoid touching user-maintained files when I do code gen, even if it's just adding something to it. And in this case, it doesn't buy the user much because they still are going to need to change the type anyway. I'm thinking that it would be awesome to have a way to give really good error messages to direct the user to exactly what's wrong.

I expected to find an Article type exposed from the TemplateData module. Try creating and/or exposing a type with that exact name.
It can have any type. For example:
type alias Article = ()
type alias Article = { title : String, description : String }
type alias Article = Data.Article.ArticleData
type Article = Draft { title : String } | Published { title : String, publishedAt : Time.Posix }

There's no great way to export types in pure Elm, but fortunately we do control the compilation process, so we can guarantee that the user will see that message and give them more nuanced information and guidance.

Maybe in the future there could be a npx elm-pages init command, similar to elm-spa init? One could then choose whether a project is generated with Html or elm-ui.

I've thought about elm-pages init. It's a tricky one because there are so many ways you can go with boilerplates. I think Gatsby init just takes a repo name, but at that point it makes me wonder what value there is beyond just cloning a repo yourself 🤔 I kind of like the idea of letting lots of boilerplate repos emerge, and not having an opinion about a built-in starter repo.

@sevfurneaux
Copy link
Contributor

I'm thinking that it would be awesome to have a way to give really good error messages to direct the user to exactly what's wrong.

Helpful error messages would be really beneficial. I think that would be a nice onboarding process when a user starts to add Template files to avoid confusion. A way of educating how TemplateData should work.

I kind of like the idea of letting lots of boilerplate repos emerge, and not having an opinion about a built-in starter repo

Yeah, I see what you mean. Ultimately that’s more helpful than an opinionated built-in starter repo.

@dillonkearns
Copy link
Owner Author

dillonkearns commented Nov 6, 2020

I think this feature is almost ready to lose the term "Beta" and turn into a standard feature! Progress so far:

Done

  • Generate Template Modules based on single source of truth from types defined in TemplateTypeDefs.elm, and generate the variants based on that in TemplateType.elm
  • Don't require user to define/expose type alias Msg = Never and type Model = () if they're using a template with no state
  • No more compiler error in generated code for exactly one Template Module (no more wildcards generated in this case)

TODO Before Template Modules Release Candidate

  • Report errors when building a site when the generate script needs to be run
  • Find a better way to do the type alias SharedTemplate - right now it's in user-maintained code, Shared.elm, which is confusing. But extracting it leads to lots of type variables and circular imports.

Future iteration

  • Wire in StaticPayload to more places (init, update, and in the Site module)

Other feedback?

I'm still looking to hear people's thoughts and experiences with this, so feel free to comment and continue the discussion here!

@dillonkearns
Copy link
Owner Author

Release Candidate Process

Just to clarify a few things, this is what I'm envisioning for doing a release candidate for Template Modules.

  • Ship a version of the NPM package that generates the main Template Module entrypoint file the way it will be in the final version (maybe something exposed from Pages.elm?)
  • Give people a window of time to migrate (so the current non-Template Modules version AND the new Template Modules approach will both work in the release). Call the old approach officially deprecated.
  • Then the next release will remove the deprecated version, and Template Modules will be the new approach going forward.

@sevfurneaux
Copy link
Contributor

Sounds great 💪

@dillonkearns
Copy link
Owner Author

All the good stuff I learned from this experiment has now made its way into the 2.0 release! 🎉 Thanks for all the good feedback!

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

No branches or pull requests

2 participants