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

Allow _meta patterns to support any engine #657

Closed

Conversation

AndrewLeedham
Copy link
Contributor

Addresses: _meta patterns being forced to use mustache.

Summary of changes:

  • Replaced ".mustache" literal with patterlab.config.patternExtension.
  • Allows the use of other engines like handlebars for _meta files.

@bramsmulders
Copy link

Great idea, but this wouldn't work in some engines. For instance the react engine cannot spit out an incomplete dom tree.

@dave-cross
Copy link

Curious if this _meta section would be a better fit in pattern engines themselves. This should allow engines to have better control over their respective file formats. A single method in the engine that either calls multiple meta files or one (react) wrapper file.

Possible PL3.0 wish?

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented May 4, 2017

@cloudsociety
you are right on target. 🎯

this is basically the idea behind #611 which is planned for 3.0

I refrained from commenting on this PR until I could think through how (or if at all) patterlab.config.patternExtension factors into that solution

@AndrewLeedham
Copy link
Contributor Author

This was more of a hot fix for a project I'm working on, that I thought I'd create a PR for, so I didn't consider all outcomes. I also noticed that the styleguidekit extensions are fixed as .mustache, so I can't use styleguidekit-handlebars-default.

@dave-cross
Copy link

@bmuenzenmeyer Awesome! Looking forward to it!

@AndrewLeedham I'll still steal this bit of code so my patterns can build under a single engine in 2.x (having too much fun with Twig/Nunjucks to return to Mustache). Thanks for the share.

-Dave

@bmuenzenmeyer
Copy link
Member

since this is in the spirit of what we want to do, but the wrong implementation, i am going to close this for now. PL Node 3 will handle this with my above approach

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

Successfully merging this pull request may close these issues.

4 participants