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

Broken with handlebars 4.6.0 #58

Closed
darthsteven opened this issue Jan 9, 2020 · 11 comments
Closed

Broken with handlebars 4.6.0 #58

darthsteven opened this issue Jan 9, 2020 · 11 comments
Labels

Comments

@darthsteven
Copy link

Handlebars 4.6.0 breaks the templating as far as I can tell.

Presumably because of the breaking change in: handlebars-lang/handlebars.js#1633

I have a custom template that extends the spritesheet-template and now it's compiled to a file of mostly comments:

// SCSS variables are information about icon's compiled state, stored under its original file name
//
// .icon-home {
//   width: $icon-home-width;
// }
//
// The large array-like variables contain all information about a single icon
// $icon-home: x y offset_x offset_y width height total_width total_height image_path;
//
// At the bottom of this section, we provide information about the spritesheet itself
// $spritesheet: width height image $spritesheet-sprites;
$: ;
$: ;
$: '';
$: ();
$: (, , '', $ );
$: ;
$: ;
$: '';
$: ();
$: (, , '', $ );
// These "retina group" variables are mappings for the naming and pairing of normal and retina sprites.
//
// The list formatted variables are intended for mixins like `retina-sprite` and `retina-sprites`.
$: ();

Obviously that's not great!

I'm not really a node dev so can't really see what's going on and how to fix it, but I can confirm that installing handlebars 4.5.3 allows the template to compile fine.

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Looking into this

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Yep, seeing same behavior as reported via npm test (works for handlebars@4.5.3., breaks for 4.6.0)

The linked issue seems unrelated but will keep on reading for a bit before recommending hardcoding handlebars version

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

They did a huge code cleanup between 4.5.3 and 4.6.0 (10k lines) using automated tools like eslint and prettier

handlebars-lang/handlebars.js@v4.5.3...v4.6.0

I've got a feeling the issue lies in that rather than the prototype code. Going to use git bisect to see if I can narrow down the cause on my own. Otherwise, I'll likely submit a bug report with a gist test case for them to use

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Actually... going to cut a release for using ~4.5.3 instead of ^ to immediately resolve this. I'd prefer to not rely on them their schedule for a patched release. I'll continue to explore the bug report route one way or another though

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Alright, that fixed version has been released in 10.4.3. Thanks for the bug report!

@darthsteven
Copy link
Author

Amazing! Thank you!

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Heh, it turns out that the issue was in handlebars-layouts incompatibility which has been fixed between its 1.x.x and 3.x.x release. Going to release another version of spritesheet-templates with the latest handlebars and handlebars-layouts

@twolfson
Copy link
Owner

twolfson commented Jan 9, 2020

Alright, this has been released in 10.5.0. Thanks again for the bug report =)

@twolfson twolfson closed this as completed Jan 9, 2020
@twolfson twolfson added the bug label Jan 9, 2020
@darthsteven
Copy link
Author

Thank you again!

@nknapp
Copy link

nknapp commented Jan 10, 2020

We will add an option to allow proto access in 4.7.0, which I hope to release today

@nknapp
Copy link

nknapp commented Jan 10, 2020

Handlebars 4.7.0 has been release with options to disable prototype restrictions:
https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access

Please be aware the you are potentially opening up ways to crash your servers or maybe even remote code execution for people who can inject templates into your system.

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

No branches or pull requests

3 participants