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

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

Open
mjjo53 opened this issue Dec 12, 2019 · 4 comments
Open
Labels
bug Something isn't working
Milestone

Comments

@mjjo53
Copy link

mjjo53 commented Dec 12, 2019

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>
@mjjo53 mjjo53 added the bug Something isn't working label Dec 12, 2019
@thibaudcolas
Copy link
Collaborator

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 thibaudcolas added this to the Nice to have milestone Dec 13, 2019
@thibaudcolas thibaudcolas changed the title DOM class variable DOM class variable issue with multiple engines (draftjs_exporter_markdown) Dec 24, 2019
@thibaudcolas
Copy link
Collaborator

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
Copy link

bindung commented Jul 2, 2020

@thibaudcolas
Your example code is not thread safe.

@thibaudcolas
Copy link
Collaborator

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants