kentcdodds commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

amwmedia commented 8 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

yep that makes sense!

benmvp commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@amwmedia any thoughts on this?

amwmedia commented 6 years ago

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

amwmedia commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

amwmedia commented 6 years ago

great, thanks!

benmvp commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

Looks like we both forgot about this 😉

amwmedia commented 6 years ago

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 commented 6 years ago

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

marcelmokos commented 5 years ago

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 commented 5 years ago

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

yoosif0 commented 5 years ago

Any news about using ejs?

yoosif0 commented 5 years ago

I started using hygen. Thank you

crutchcorn commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

benmvp commented 4 years ago

That's what I said 😂

smolinari commented 1 week ago

Just want to politely note that I too would very much prefer to use EJS above hbs, because it is a much better templating engine and found this issue in the end and was hoping plop could support EJS. Now looking at hygen, as it has been noted several times. 🤷 But, as an overall note, I was impressed at the work done to make plop and its extensive API. Well done!

Scott

plopjs / plop

Extract template language and data gathering #10

`setGeneratorDefaults()`

`templateRenderer`

`helpers`

`partials`

`templateRenders`

`setGenerator()`

`templateRenderer`

`helpers`

`partials`

`handlebars`

`setHelper`

`getHelper` / `getHelperList`

`setPartial`

`getPartial` / `getPartialList`

Example plopfile

New `plop.apiMethods()`

plopjs / plop

Extract template language and data gathering #10

setGeneratorDefaults()

templateRenderer

helpers

partials

templateRenders

setGenerator()

templateRenderer

helpers

partials

handlebars

setHelper

getHelper / getHelperList

setPartial

getPartial / getPartialList

Example plopfile

New plop.apiMethods()

`setGeneratorDefaults()`

`templateRenderer`

`helpers`

`partials`

`templateRenders`

`setGenerator()`

`templateRenderer`

`helpers`

`partials`

`handlebars`

`setHelper`

`getHelper` / `getHelperList`

`setPartial`

`getPartial` / `getPartialList`

New `plop.apiMethods()`