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

Extract template language and data gathering #10

Open
kentcdodds opened this issue Dec 10, 2015 · 39 comments
Open

Extract template language and data gathering #10

kentcdodds opened this issue Dec 10, 2015 · 39 comments

Comments

@kentcdodds
Copy link
Contributor

I think that it'd make this utility more powerful if both inquirer and handlebars integration were extracted to plugins to make this utility pluggable. That would allow me to provide my own template compiler and data gatherer. It would also allow for customizing these things without having to make plop expose APIs for these things explicitly. Thoughts?

@amwmedia
Copy link
Member

I could see the benefit to using another template engine, but what other types of data gatherers would you envision being useful? My initial feeling is that if you need something THAT custom, you could just write a custom node script that would do the job.

@kentcdodds
Copy link
Contributor Author

Perhaps you're right. Honestly the only use case I had when I initially thought of this was when plop didn't support partials and I wanted to add one, there was no way to do this. Now that it supports that I don't have a use case for this capability.

At the same time I could definitely see that some people may wish to utilize plop with a different template engine. As for the data gathering tool, there are others, but inquirer is pretty good and I certainly don't see a reason to use anything else.

The other thing is there's a great benefit to making tools pluggable so others could develop plop plugins themselves (freeing up your personal time which I'm sure you're beginning to notice is slipping slowly away) :-)

Also, thanks for pushing back :-) PDD

@amwmedia
Copy link
Member

yeah, I've tried to do that a little recently with custom action functions which really open up a lot of possibilities for the JS guru. It would be nice to have an extension point on the input side too, but I think maybe an inquirer addPrompt function may fit the bill.

I like the idea of making things pluggable, but I also want to keep things simple for the 90% case.

@kentcdodds
Copy link
Contributor Author

Personally I think that it can be pluggable and simple(r) but it's your project (and thank you for it!)

@amwmedia
Copy link
Member

possibly, if you think of a simple way to do it let me know. I guess I'm not really sure what benefit it would provide. inquirer and handlebars are both pretty pluggable, so I'm not sure what else you might need. I'm sure there are things I'm just not thinking of.

@benmvp
Copy link

benmvp commented Dec 21, 2017

Two year old issue, but I'll jump in anyway. I was gonna create a separate issue, but this seems relevant. I wanted to ask if you ever considered moving away from Handlebars into something like EJS?

Handlebars' built-in plug-ins are so limited it makes my template code so messy. I'd rather not have to create a custom plugin just to compare 2 values or to do an || expression. EJS seems nice because you can just use normal JS w/in the templates.

This change would totally not be backwards compatible and switching over would probably be problematic given how different the templating approaches are, but if the templating part was pluggable as @kentcdodds was suggesting, then it could be a win-win?

What do ya think?

@amwmedia
Copy link
Member

I'll give it some thought. I wouldn't be opposed to creating a path where someone could plug in a different template engine, but I would probably want to keep Handlebars as the default for simplicity. There might be a way to provide hooks to basically bypass Handlebars and pipe the template data into another system... 🤔

@benmvp
Copy link

benmvp commented Dec 22, 2017

Yeah I think it's wise to keep Handlebars as a default.

In trying to get passed Handlebars' limitations a did a lot of code spelunking in plop & node-plop, so I could look into what it would look like to configure plop to use a different templating language. Hopefully I'll have time to investigate.

Thanks!

@amwmedia
Copy link
Member

@benmvp that would be great. I'm thinking that we might be able to add a plop api method that would register a function to take over template rendering (something like setTemplateRenderer). Plop could basically call that renderer method with the template and data object if it is present and bypass handlebars altogether.

Thoughts?

@amwmedia
Copy link
Member

Another possibility would be to simply add a lifecycle hook that would execute prior to template rendering. Handlebars could still process the result, but if there's nothing for handlebars to do, then the data would simply pass through.

@benmvp
Copy link

benmvp commented Dec 28, 2017

Yep the setTemplateRenderer was what I was thinking as well. Nice!

What's the advantage of having Handlebars process after the lifecycle hook? I'm not seeing the use-case where you'd want double processing. Is it just the limit the scope of the change of supporting a different renderer?

@amwmedia
Copy link
Member

My thought was that there might be a desire to use EJS without breaking existing Handlebar generators. By keeping the handlebar processing, a plopfile could be migrated to a new rendering engine over time.

Maybe the ideal setup is one that allows the rendering engine method to be a config attribute on the setGenerator() object, rather than forcing it to be set globally for the whole plopfile. hmmm...

@benmvp
Copy link

benmvp commented Dec 28, 2017

Good point. We don't want it as easy as possible for folks to migrate over. Having it as a config property on setGenerator() means you could still have your existing generators & templates running on handlebars, but add a new generator that uses EJS. Then slowly transition (as necessary).

I would be annoying though if you're starting fresh (like I was 2 weeks ago), and you had to specify EJS for each generator. Maybe the answer is a global setting for the whole plopfile with the ability to override on the generator level? So if you wanted EJS for everything, the plopfile could just define that. But if you've got existing stuff, you can go in one by one for each generator. Does that work?

@amwmedia
Copy link
Member

I wonder if a more generic way to set defaults might be the way to go. We could use something like plop.setGeneratorDefaults({ ... }) to set a base configuration that all generators would inherit from. That would make future configurations like this pretty simple.

@benmvp
Copy link

benmvp commented Dec 29, 2017

yep that makes sense!

@benmvp
Copy link

benmvp commented Dec 30, 2017

I'll put together an updated API draft + a high-level spec of the necessary changes here and in node-plop to run by you. Then if you give the 👍, I'll work on an implementation.

@benmvp
Copy link

benmvp commented Jan 11, 2018

Ok, sorry for the delay. Got back from break this week and was able to start thinking through the API. It looks like the entirety of the API is in node-plop, with plop just being the CLI runner.

This is just an initial stab at it, so I'd love your feedback. Let me know if you think there's a better environment to work through this


setGeneratorDefaults()

New method that sets a base configuration from which all generators would inherit.

templateRenderer

Object property that allows for setting a default template renderer for all generators.

Accepts the following properties:

  • render(template: String, data: Object, helpers: Object, partials: Object): String: Given a template string, a data object and optional helpers & partials objects, returns the rendered template.
    • Defaults to using templateRenderers.handlebars configuration (see below).
    • One nice thing about exposing render is that it provides a way to inject more things into data, such as computed values.
  • setHelper / getHelper / getHelperList: work like plop.* equivalents to operate on helpers that will apply to all generators (see below)
    • They will be merged with the "baked-in" helpers when passed to render().
    • Defaults to something similar to the current implementations
    • Likely to not be overriden, so they could just be defined internally (like they are now) and not be made available for overriding
  • setPartial / getPartial / getPartialList: work like plop.* equivalents to operate on partials that will apply to all generators (see below)
    • Handlebars-specific
    • Defaults to something similar to the current implementations
    • Likely to not be overriden, so they could just be defined internally (like they are now) and not be made available for overriding

helpers

A newly exposed object lookup of global helpers (similar to the internal representation of helpers). A declarative way to specify the globally-defined helpers.

partials

A newly exposed object lookup of global partials (similar to the internal representation of partials). A declarative way to specify the globally-defined helpers.

templateRenders

A read-only object of default render() functions for common template renderers (such as handlebars & ejs).

  • handlebars - default render() for handlebars template renderer. To be set up just like current setup for backwards compatibility
  • ejs - default configuration setting for ejs template renderer. See below for a sample setup

setGenerator()

Existing method that now allows for overriding the default template renderer for a specific genrator.

templateRenderer

Object property that allows for setting the template renderer for this specific generator.

Accepts the following properties:

  • render(template: String, data: Object, helpers: Object, partials: Object): String: Overrides the default render() for the specific generator
    • Defaults to using the one defined in setGeneratorDefaults

helpers

An object lookup of helpers for the specific generator (similar to the globally-defined ones). Open question as to whether the helpers defined here on the generator level should override any globally-defined ones. Presumably the renderer you're overriding here is different than the global one, so merging in helpers wouldn't make sense. However, the "baked-in" helpers are generic enough to always merge in to render().

partials

An object lookup of partials (similar to the globally-defined ones), Open question as to whether the partials defined here on the generator level should override any globally-defined ones. Presumably the renderer you're overriding here is different than the global one, so merging in partials wouldn't make sense. However, the "baked-in" partials are generic enough to always merge in to render().

handlebars

Not quite sure how this is being used. It probably should be deprecated, but it's just a reference to handlebars so it's not the end of the world either.

setHelper

The imperative way to specify globally-defined helpers. Would merge into the helpers defined via helpers within setGeneratorDefaults when used, but the new declarative helpers would be preferred.

getHelper / getHelperList

Operate the same on the globally-defined helpers. These are undocumented, so not sure how these were being used.

setPartial

The imperative way to specify globally-defined partials. Would merge into the helpers defined via partials within setGeneratorDefaults when used, but the declarative new partials would be preferred. (Handlebars-specific)

getPartial / getPartialList

Operate the same on the globally-defined partials. (Handlebars-specific) These are undocumented, so not sure how these were being used.

Example plopfile

import ejs from 'ejs';
import handlebars from 'handlebars';

export default (plop) => {
    plop.setGeneratorDefaults({
        templateRenderer: {
            render: (template, data, helpers) => (
                // NOTE: no need for partials because EJS handles those w/ includes
                ejs.render(
                    template,
                    Object.assign({}, helpers, data),
                    {debug: true}
                )
            ),
        },
    })

    plop.setGenerator('name', {
        templateRenderer: {
            render: (template, data, helpers, partials) => {
                Object.keys(helpers).forEach(h => handlebars.registerHelper(h, helpers[h]));
                Object.keys(partials).forEach(p => handlebars.registerPartial(p, partials[p]));
                return handlebars.compile(template)(data);
            },
        },
    })

    // -or-

    plop.setGenerator('name', {
        templateRenderer: plop.templateRenderers.handlebars,
    })
}

@benmvp
Copy link

benmvp commented Jan 26, 2018

@amwmedia any thoughts on this?

@amwmedia
Copy link
Member

@benmvp sorry for the delay, I'm ready over this and will respond soon.

@amwmedia
Copy link
Member

ok, I think I have an idea of how we can do this and introduce a little less mental overhead.

New plop.apiMethods()

  • plop.setGeneratorDefaults(configObject): As you described above, a method to define a set of default values for any property that the generator config object supports
  • plop.getGeneratorDefaults(): gets the defined default config object.
  • plop.setTemplateRenderer(name, renderFunction(templateString, data, plop)): Registers a new template renderer method by name. How the rendering happens is entirely up to the renderFunction but the plop param can be used to access helpers or partials if desired. By registering a template renderer via
    • A templateRenderer property on the generator config would reference the renderer by name. This would follow the pattern of generators, helpers, actionTypes, etc which would make it pretty simple to abstract behind plop.load. So then an EJS renderer could be published as a node module and imported into any project.
  • plop.getTemplateRenderer(name): gets a template renderer method by name
  • plop.getTemplateRendererList(): gets a list of all template render names

thoughts? am I missing anything?

@benmvp
Copy link

benmvp commented Jan 31, 2018

I think this looks good! I'm assuming if you don't specify templateRenderer on the generator config you get the default handlebars?

I really like passing in plop to the renderFunction which allows you to basically get any info you want.

So rewriting my sample plopfile from above would look like:

import ejs from 'ejs';

export default (plop) => {
    plop.setTemplateRenderer('ejs', (template, data, plop) => {
        // need to build object lookup of helpers to pass to ejs.render()
        const helperList = plop.getHelperList();
        const helpers = helperList.reduce((helpersLookup, helperName) => {
            helpersLookup[helperName] = plop.getHelper(helperName);
        }, {})

        // NOTE: no need for partials because EJS handles those w/ includes
        return ejs.render(
            template,
            Object.assign({}, helpers, data),
            {debug: true}
        )
    });

    plop.setGenerator('name', {
        templateRenderer: 'ejs',
        description: 'A generator based on EJS',
        prompts: {

        },
        actions: {

        }
    });

    // -or-

    plop.setGenerator('name', {
        description: 'A generator based on the default Handlebars',
        prompts: {

        },
        actions: {
            
        }
    });
}

The only thing funky is the helpers part. The interesting thing is that the library has partials as the object lookup, but the API only exports the list of names via getHelperList(). Potentially we expose plop.getHelpers() & plop.getPartials()?

@amwmedia
Copy link
Member

yeah, I could see these get methods being added for all of the main data types (generators, helpers, partials, actionTypes, and templateRenderers). Definitely saves a step when you need to work with that data.

@amwmedia
Copy link
Member

your modified example looks good, but (in reality) the eventual usage would probably look more like...

export default (plop) => {
    plop.load('plop-ejs');

    plop.setGenerator('name', {
        templateRenderer: 'ejs',
        description: 'A generator based on EJS',
        prompts: [],
        actions: []
    });

    // -or-

    plop.setGenerator('name', {
        // default templateRenderer: 'handlebars',
        description: 'A generator based on the default Handlebars',
        prompts: [],
        actions: []
    });
}

@benmvp
Copy link

benmvp commented Jan 31, 2018

Ahhh... I see. Hadn't really investigated plop.load(). So the setTemplateRenderer() part would go in a new NPM package called plop-ejs:

export default function (plop) {
    plop.setTemplateRenderer('ejs', (template, data, plop) => {
        // need to build object lookup of helpers to pass to ejs.render()
        const helperList = plop.getHelperList();
        const helpers = helperList.reduce((helpersLookup, helperName) => {
            helpersLookup[helperName] = plop.getHelper(helperName);
        }, {})

        // NOTE: no need for partials because EJS handles those w/ includes
        return ejs.render(
            template,
            Object.assign({}, helpers, data),
            {debug: true}
        )
    });
};

?

@benmvp
Copy link

benmvp commented Jan 31, 2018

I think we've got the API in a solid place, so I'll start dev on it.

@amwmedia
Copy link
Member

amwmedia commented Feb 1, 2018

great, thanks!

@benmvp
Copy link

benmvp commented Feb 13, 2018

Hey @amwmedia! So we missed one big part of this and that's renderString(template, data). As you're no doubt aware, it's used by everything and now we'd need it to work w/in the context of a template renderer.

I'm thinking we can update it to take an optional third templateRenderer property which will use that template renderer, otherwise it'll use the default. We can start off w/ most of the operations just using the default, but the actions-related calls really need to use the one defined on the generator. And that means they'll need the generator template renderer passed to them (here and here).

Does that sound about right?

@amwmedia
Copy link
Member

@benmvp hmmmm, I'll look into this in more detail today. I actually had a moment there where something inside me said "that was almost... too easy". 😐

@benmvp
Copy link

benmvp commented Apr 9, 2018

Looks like we both forgot about this 😉

@amwmedia
Copy link
Member

it's still on "my list" but unfortunately, my calendar blew up at work and home so I haven't been able to find much time. I suppose we would need to handle this the same way as we would for the generators. Like you say, we could accept a 3rd (optional) param that would be the templateRenderer value (ie "ejs") but would default to "handlebars" if unspecified. The renderString function would need to lookup the renderer within the plop context and run it.

@benmvp
Copy link

benmvp commented Apr 10, 2018

👍 I'll just try to put something together and we can work from there

@marcelmokos
Copy link

If you need any help I am willing to help with the change.
I had a similar idea and I opened issue yesterday in node-plop https://github.com/amwmedia/node-plop/issues/101.

Now I see that it is duplicate and I really want to introduce this change.

@benmvp
Copy link

benmvp commented Feb 18, 2019

I'm in favor of you taking it over @marcelmokos!

@yoosif0
Copy link

yoosif0 commented Jun 25, 2019

Any news about using ejs?

@yoosif0
Copy link

yoosif0 commented Jun 26, 2019

I started using hygen. Thank you

@crutchcorn
Copy link
Member

@benmvp have you ended up starting work on this? If so, is there a place we could look at the changes that were made to get things started on it?

@benmvp
Copy link

benmvp commented Feb 15, 2020

Nope I didn't. I needed some help from @amwmedia but after the delays just moved on. I don't really remember any of the details 2 years later. Still think it's a good idea so have at it!

@crutchcorn
Copy link
Member

No worries, I just wanted to check. I'll see what I can do about implementing this in the coming months

@benmvp
Copy link

benmvp commented Feb 16, 2020

That's what I said 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants