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

Accept a function as options.name #199

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wiggisser
Copy link

Accept a function as options.name to be able to specify the templates name based on the name of the input file

index.js Outdated
@@ -16,6 +17,9 @@ module.exports = function gulpPug(options) {
const data = Object.assign({}, opts.data, file.data || {});

opts.filename = file.path;
if (typeof namefunc === 'function') {
opts.name = namefunc(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should move this into the opts.client conditional because it is only used for client compilation. We might as well avoid extra loops when it doesn't effect the output.

index.js Outdated Show resolved Hide resolved
@demurgos
Copy link
Member

demurgos commented May 9, 2019

The type definitions should also be updated, but you no longer can directly extend Pug's options because on the pug side, name must be a string. You can leave it to me if you are not familiar with Typescript, I'll fix it before merging.

@wiggisser
Copy link
Author

fixes #192

index.js Outdated Show resolved Hide resolved
@demurgos
Copy link
Member

demurgos commented May 9, 2019

The current code should work fine. My main issue was with the variable name, you now check its type so it's less confusing.
I was expecting to avoid the temporary variable or possibly undefined options with:

if (typeof opts.name === "function") {
  const name = opts.name(...);
  compiled = pug.compileClient(contents, Object.assign({}, opts, {name}));
} else {
  compiled = pug.compileClient(contents, opts);
} 

Thanks for the PR and tests. I'll add a changelog entry, update the type definitions, documentation and rebase on top of #201. It will be published to npm soon (probably with #200).

@lcavadas
Copy link

+1

Any chance this will be released/published soon?

@phated
Copy link
Member

phated commented Jun 9, 2021

@wiggisser @lcavadas if either of you want to rebase this PR on the latest changes and update the type definitions, I'm happy to get it released.

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

Successfully merging this pull request may close these issues.

4 participants