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

feat: Add partialsDir.rename option #151

Merged
merged 2 commits into from Mar 30, 2021
Merged

Conversation

djaax
Copy link
Contributor

@djaax djaax commented Mar 26, 2021

We need more flexibility when it comes to naming partials and don't want to be constrained by our folder structure.

For example, we have

/components
    /button
        button.hbs
    /template
        otherTemplate.hbs

Currently, this names the partial button/button and template/otherTemplate.

This renaming function allows more flexibility, e.g., naming the two templates button and otherTemplate, ignoring the folder structure.

We need more flexibility when it comes to naming partials and don't want to be constrained by our folder structure.

For example, we have

/components
    /button
        button.hbs
    /template
        otherTemplate.hbs
Currently, this names the partial button/button and template/otherTemplate.

This renaming function allows more flexibility, e.g., naming the two templates button and otherTemplate, ignoring the folder structure.
@UziTech
Copy link
Member

UziTech commented Mar 26, 2021

Currently, this names the partial button/button and template/otherTemplate.

Why are those not good names? Is there a reason you need "better" names?

@djaax
Copy link
Contributor Author

djaax commented Mar 26, 2021

Continuing the example above, we want to have the template next to other files, e.g. a JS or CSS file organized in subfolders in components. So, next to button.hbs there could be button.js, button.css.

Without the renaming function, the partial's name would be 'button/button' which is redundant and doesn't work with our implementation. Without the renaming feature, we'd have to keep all templates in one partials dir.

@UziTech
Copy link
Member

UziTech commented Mar 26, 2021

'button/button' which is redundant and doesn't work with our implementation

why doesn't it work with your implementation?

Why is is being redundant a bad thing? It shows the path to the partial so it is easy to find.

I'm trying to understand if this is a "need" or a "want". Or if there is a better way to accomplish the "want".

This doesn't seem like a feature that would be very useful to many users so it would be better if there is a way to handle it without causing feature creep

image

@djaax
Copy link
Contributor Author

djaax commented Mar 26, 2021

I understand your concern. I try to explain why we want/need it and you can decide if it potentially helps others or if it qualifies as feature creep.

We're using things like {{> (lookup . 'type') this }} which automatically loads the partial. In the configuration type is, e.g., button (not button/button).

Renaming the partial would be the most elegant solution for us. But sure, there are workarounds, mapping the type with the partial path, etc.

Again, if you deem this unnecessary feel free to ignore this PR.

@UziTech
Copy link
Member

UziTech commented Mar 28, 2021

Do you have a workaround that currently works?

@djaax
Copy link
Contributor Author

djaax commented Mar 29, 2021

Well, yes, not organize handlebar files/partials in component folders but one single partial folder. Not ideal but it's ok.

@UziTech UziTech changed the title feat(partials): Optional (re)name partials functionality feat: Add partialsDir.rename option Mar 30, 2021
@UziTech UziTech merged commit 1a6771b into express-handlebars:master Mar 30, 2021
github-actions bot pushed a commit that referenced this pull request Mar 30, 2021
# [5.3.0](v5.2.1...v5.3.0) (2021-03-30)

### Features

* Add partialsDir.rename option ([#151](#151)) ([1a6771b](1a6771b))
@github-actions
Copy link

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@UziTech
Copy link
Member

UziTech commented Mar 30, 2021

Thanks for this! Sorry it took me so long to review.

@djaax
Copy link
Contributor Author

djaax commented Mar 30, 2021

Thank you, I appreciate it!

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

Successfully merging this pull request may close these issues.

None yet

2 participants