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

(Breaking) Change in register_templates_directory #549

Closed
Nerixyz opened this issue Dec 19, 2022 · 2 comments · Fixed by #551
Closed

(Breaking) Change in register_templates_directory #549

Nerixyz opened this issue Dec 19, 2022 · 2 comments · Fixed by #551
Assignees
Labels

Comments

@Nerixyz
Copy link

Nerixyz commented Dec 19, 2022

Upgrading from version 4.2.2 to 4.3.0, specifically because of #500 (580acba), might result in register_templates_directory not working identically.

In v4.2.2, you could register files with an extension that contained multiple .'s:

handlebars.register_templates_directory(".hbs.html", "templates");

This would result in all files ending in .hbs.html to be loaded. After upgrading, this breaks, as now, Path::extension is used to determine the extension:

.filter(|tpl_path| {
tpl_path
.extension()
.map(|extension| extension == tpl_extension)
.unwrap_or(false)
})

Thus, in the example above, no files would ever match, since Path::extension considers html as the extension.

The documentation doesn't mention anything regarding the format of extension.

If my use-case is invalid, then the docs should at least mention that extension must never contain a . (excluding the first character), or that Path::extension will be used to determine the extension. Furthermore, the changelog could mention, that this previously accepted but invalid use will no longer work.

@sunng87 sunng87 self-assigned this Dec 20, 2022
@sunng87 sunng87 added the bug label Dec 20, 2022
@sunng87
Copy link
Owner

sunng87 commented Dec 20, 2022

hello @Nerixyz , your use case should be accepted by handlebars registry. Unfortunately our tests did not cover this case. Let's treat it as a bug and I'm going to fix it in 4.3.x releases.

@sunng87
Copy link
Owner

sunng87 commented Dec 21, 2022

@Nerixyz The fix is released in 4.3.6

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 a pull request may close this issue.

2 participants