springload / draftjs_exporter

Convert Draft.js ContentState to HTML
https://www.draftail.org/blog/2018/03/13/rethinking-rich-text-pipelines-with-draft-js
MIT License
83 stars 21 forks source link

DOM class variable issue with multiple engines (draftjs_exporter_markdown) #122

Open mjjo53 opened 4 years ago

mjjo53 commented 4 years ago

First of all, I would like to thank you as someone who is using this library. I'm using this library with a markdown version. (draftjs_exporter_markdown) Because the dom of draftjs_exporter.dom.DOM is a class-level variable, it could be a problem depending on the timing of creating two versions of the HTML object. Furthermore, I'm using this on a web server, this can cause bigger problems in a multithreaded environment. Not only the markdown version, but the 3 engines built-in draftjs_exporter will cause the same problem.

Could you solve this problem? Attached test code and results.

from draftjs_exporter.dom import DOM
from draftjs_exporter.html import HTML
from draftjs_exporter.constants import ENTITY_TYPES
from draftjs_exporter_markdown import ENGINE as MARKDOWN_ENGINE

template = {
    'blocks': [
        {'key': 'rrwx',
         'type': 'unstyled',
         'text': '<a href="https://www.google.com">google.com</a>',
         'depth': 0,
         'inlineStyleRanges': [],
         'entityRanges': [{'offset': 9, 'length': 22, 'key': 0}],
         'data': {}}],
    'entityMap': {'0': {'type': 'LINK',
                        'mutability': 'MUTABLE',
                        'data': {'url': 'https://www.google.com'}}}}

html_exporter = HTML({
    'entity_decorators': {
        ENTITY_TYPES.LINK: lambda props: DOM.create_element('a', {
            'href': props['url']
        }, props['children']),
    },
    'engine': DOM.LXML
})
html1 = html_exporter.render(template)

markdown_exporter = HTML({
    'engine': MARKDOWN_ENGINE
})
html2 = html_exporter.render(template)

print(html1)
print(html2)

output:

<p>&lt;a href="<a href="https://www.google.com">https://www.google.com</a>"&gt;google.com&lt;/a&gt;</p>
<p><a href="<a href="https://www.google.com">https://www.google.com</a>">google.com</a></p>
thibaudcolas commented 4 years ago

Hi @mjjo53, thanks for reporting this. Do you have some thoughts on what a fix would look like, or are you interested in contributing one? I’m interested in helping getting this working, but it will go much faster getting a fix done if you can drive this.

thibaudcolas commented 4 years ago

I just realised that I actually also ran into this issue on one of my projects, and solved it by manually re-setting the engine whenever calling render:

https://github.com/thibaudcolas/draftail-playground/blob/81ad38dd24bbc67a00ba93a751d8f4f8946d4268/markdown.py#L20-L22

def render_markdown(content_state):
    DOM.use(ENGINE)
    return exporter.render(content_state)

Does that seem like a viable solution to you? I think I might add this directly in exporter.render if so, so we can solve this without having to change how this code is structured too much.

bindung commented 4 years ago

@thibaudcolas Your example code is not thread safe.

thibaudcolas commented 4 years ago

@bindung yes, as far as I know the whole library isn’t thread-safe, as @mjjo53 initially reported. The code above only helps in a single-threaded environment. Do you have any suggestions for how to fix this properly, or further diagnostic of the problem?