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

markdownify returns stale unamended markdown library instance #696

Open
1 task done
cbutcosk opened this issue Mar 16, 2023 · 4 comments
Open
1 task done

markdownify returns stale unamended markdown library instance #696

cbutcosk opened this issue Mar 16, 2023 · 4 comments
Labels
quire-11ty This is an issue with the quire-11ty package workaround Issue has a workaround

Comments

@cbutcosk
Copy link
Collaborator

Before proceeding, check to make sure there isn’t an existing issue for this bug.

  • I have searched the existing issues and determined this is a new bug.

Steps to reproduce

  • Amend the markdown library using 11ty's amendLibrary API
  • Use markdownify to render markdown that uses the amended feature

Actual behavior

The rendered HTML does not include markdown renderer amendments.

Expected behavior

The rendered HTML should included markdown renderer amendments.

Version numbers

1.0.0-rc3

What browsers are you seeing the problem on?

No response

Relevant Terminal/Shell output

No response

Supporting Information

We worked around this by iterating the libraryAmendments array before the markdownlibrary instance is returned in markdownify, but 11ty has also lessened the need for a markdown rendering filter now that they released the renderTemplate API.

@cbutcosk cbutcosk added the status:triage needed Issue needs to be triaged by the Quire team. This label is automatically applied to new issues. label Mar 16, 2023
@geealbers
Copy link
Member

Thanks @cbutcosk. Can you elaborate a bit on your use of amendLibrary? What was it you needed the markdown to do beyond Quire's normal functionality?

@cbutcosk
Copy link
Collaborator Author

On Exhibiting O'Keeffe we modify the markdown library to allow content editors to pass data attributes on link markup to front-end components. Editors insert a data-entity-reference="true"to indicate that the link should be picked up as a data reference to an API-backed entity and processed for the index of entities, etc.

These are then harvested by walking across all content and plucking the attributed elements out of the rendered stream, using renderTemplate in the harvesting loop so that the rendering can correctly handle whatever templating language it receives from underlying shortcodes.

We had been using markdownify in dev for this harvest-render but we discovered that it was returning a stale instance rather than the renderer 11ty was actually using as we expected. renderTemplate ultimately provided a better cut of what we needed (since it renders both markdown and liquid shortcodes with whatever is set as the render library, with amendments, and with appropriate parameter overrides where necessary) but since markdownify is used extensively in quire's component tooling we ended up needing to patch it to properly handle editor markup in other locations.

amendLibrary solves a need in quire projects because customizing render behaviors is a good way to encapsulate editor and content creator intent without increasing the size of the syntax they have to manage in markdown documents. From a long-term maintenance perspective as an integrator, the only thing better than creating a shortcode is not having to do so because HTML already covers the use case (which is very often the case!).

@hbalenda
Copy link
Contributor

hbalenda commented May 2, 2023

@cbutcosk Wondering if you can provide some code for the customization you were using. I ran an extremely simple test and the markdownify filter does apply these modifications included in amendLibrary:

// .eleventy.js 
eleventyConfig.addPlugin(markdownPlugin)
eleventyConfig.amendLibrary('md', (markdownLibrary) => {
  markdownLibrary.renderer.rules.strong_open = () => `<gradoo>`
  markdownLibrary.renderer.rules.strong_close = () => `</gradoo>`
})

I'm currently working through an issue with rendering markdown footnotes inside shortcodes that's giving me a headache, and thinking through the whole template parsing flow. Replacing rid of the markdownify filter and using renderTemplate is definitely an interesting idea.

@cbutcosk
Copy link
Collaborator Author

cbutcosk commented May 3, 2023

@hbalenda The markdownify instance staling issue appeared in our case when:

  • We amended the renderer in a plugin
  • We used our blessed attribute on an intermediate component (Figure captions in our case) that use markdownify to render

Not sure if we needed both or if the latter always would have caused it.

Anyway 11ty's internal handling of amendments is pretty naive--amendments are closures, so they just iterate and execute the stack of closures on the render lib before returning it. So we did the same thing in markdownify's filter code:

  eleventyConfig.addFilter('markdownify', (content) => {
    if (!content) return ''

    for (let amendment of eleventyConfig.libraryAmendments['md'] || []) {
      amendment(markdownLibrary)        
    }      

    return !content.match(/\n/)
      ? markdownLibrary.renderInline(content)
      : markdownLibrary.render(content)
  })

Hope that helps in whatever way with your citation issue? It definitely sounds like renderTemplate might be more up your street if you're handling mixed markdown and liquid.

ETA: Adding the code to call renderTemplate since there's some ceremony to getting it on the eleventyConfig object. Remembering now that it also requires an async function, so you'd need to resolve any function coloring for callers in markdownify.

      // Use `renderTemplate` (see https://www.11ty.dev/docs/plugins/render/#rendertemplate) to render shortcodes (in our case for figures), then HTML
      let html = await eleventyConfig.javascriptFunctions.renderTemplate(post.template.frontMatter?.content,'liquid,md' ) 

@Erin-Cecele Erin-Cecele added status:needs discussion Issue needs discussion before being addressed quire-11ty This is an issue with the quire-11ty package and removed status:triage needed Issue needs to be triaged by the Quire team. This label is automatically applied to new issues. labels May 30, 2023
@Erin-Cecele Erin-Cecele added workaround Issue has a workaround status:backlog Issue is a lower priority but needs to eventually be addressed and removed status:needs discussion Issue needs discussion before being addressed labels Jun 27, 2023
@Erin-Cecele Erin-Cecele removed the status:backlog Issue is a lower priority but needs to eventually be addressed label Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quire-11ty This is an issue with the quire-11ty package workaround Issue has a workaround
Projects
None yet
Development

No branches or pull requests

4 participants