foundation / panini

A super simple flat file generator.
Other
592 stars 104 forks source link

Marked helpers crashing in 1.7.0 #220

Closed khawkins98 closed 3 years ago

khawkins98 commented 3 years ago

Saw the new update 🎉

It looks like the {{#markdown}} text {{/markdown}} helpers are having issues though.

https://github.com/foundation/panini/blob/7dee6efde38810494ccae1d9dd966930ba82b72c/helpers/markdown.js#L16

[15:02:53] Error: Panini: rendering error occured.
Error: Unknown language: ""
Please report this to https://github.com/markedjs/marked.
    at Panini.render (/path/node_modules/panini/lib/render.js:80:11)

I was able to workaround by changing the line to:

     if (typeof language === 'undefined' || language == '') language = hljs.getLanguage(language) ? language : 'html';

But not sure I should be updating my helpers or doing something else instead?

DanielRuf commented 3 years ago

Hi @khawkins98,

thanks for reporting this. Is this probably due to the spaces? Probably a breaking change in marked. We should not simply upgrade dependencies which is a breaking change @joeworkman

https://github.com/foundation/panini/commit/51ee6dbdae5cf9f19a4738843629832c019e88ba

https://github.com/markedjs/marked/blob/master/src/Renderer.js#L15 https://github.com/markedjs/marked/blob/v0.3.4/lib/marked.js#L990-L991

0.7.0 was the last version with this parameter and logic: https://github.com/markedjs/marked/blob/v0.7.0/lib/marked.js

0.8.0 was the first release which did not use this language parameter anymore: https://github.com/markedjs/marked/blob/v0.8.0/lib/marked.js https://github.com/markedjs/marked/blob/v0.8.0/src/Renderer.js#L15

I would say we revert and pin this to 0.7.0, release it as bugfix and then prepare a new release with a new major version and use the new API.

khawkins98 commented 3 years ago

Thanks @DanielRuf

I now realise it's not the {{#markdown}} that was giving issues but my ``` So I think it might be with highlight.js

That is, this fails:

my code
```

This passes:
```html

my code

joeworkman commented 3 years ago

@khawkins98 I was just typing my response when your update popped in. I have noticed that highlight.js requires that you define a language now. It used to default to HTML. I will look into why the default is not getting triggered.

khawkins98 commented 3 years ago

Thanks @joeworkman

Also linking to https://github.com/foundation/panini/commit/8275f4e62f8ea2953de6b90cb51ef2df05a66860 -- for my own notes.