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

Do not wipe out context.data, breaking @key, @index, etc in partials #49

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

justinph
Copy link
Contributor

@justinph justinph commented Jul 23, 2016

This fixes the problems addressed in #22 and #41.

In the code without my patch, options.data is set to undefined, and then a few lines later we're merging all these objects, which causes the newly undefined options.data to wipe out any good object that we might have. As far as I can tell by tracing where options.data comes from and is used again, setting it to undefined again serves no useful purpose.

Big thanks to @yongtw123 for pointing me in the right direction in #22.

@nessthehero
Copy link

Can this pull request be merged? It fixes a pretty significant issue that limits a lot of functionality. It seems like this repo is a bit stale.

@jonschlinkert
Copy link
Member

I think so, @doowb you ok with this?

@doowb
Copy link
Member

doowb commented Aug 29, 2016

Yeah looks good.

@jfrantzius
Copy link

Do I get it right that this will likely fix #22 ?
If yes, then it would be totally great if this fix got merged, as in our experience #22 is a blocker for using grunt-assemble with handlebars!

@justinph
Copy link
Contributor Author

justinph commented Sep 6, 2016

@jfrantzius: Yes, this should fix the issues with key, index, first, etc.

@doowb
Copy link
Member

doowb commented Sep 6, 2016

@justinph thanks for this fix. I've merged it in locally and will be pushing it up shortly.
I'll double check to see if this fixes #22 also. If not, we might have to bump Handlebars in assemble-handlebars.

@doowb doowb merged commit 8710e38 into assemble:master Sep 6, 2016
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.

5 participants