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

Whitespace prevents random component to be interpreted #161

Closed
Jonarod opened this issue Mar 10, 2018 · 4 comments
Closed

Whitespace prevents random component to be interpreted #161

Jonarod opened this issue Mar 10, 2018 · 4 comments

Comments

@Jonarod
Copy link
Contributor

Jonarod commented Mar 10, 2018

Say my render logic is like this:

render((
    <Markdown
        children={md}
        options={{
            overrides: {
                MyComponent: {
                    component: MyComponent,
                },
            },
        }} />
), document.body);

MyComponent being as simple as possible like:

const MyComponent = props => <h1>{props.say}</h1>

Now, the problem occurs depending on the markdown input syntax. Observe those two similar inputs:

Input 1 (equal sign with spaces):

Say hello: <MyComponent say = "hello" />

Input 2 (equal sign withOUT spaces):

Say hello: <MyComponent say="hello" />

Input 1 outputs nothing, while Input 2 correctly prints the component's content.
Is there any way to make markdown-to-jsx more resilient to whitespaces so that both syntax works ?
I have been struggling like 3 hours just to understand where was my mistake while it was just a space messing... :/
Anyway, nice library :)

@Jonarod
Copy link
Contributor Author

Jonarod commented Mar 10, 2018

After further investigations, it seems a more global problem:

These markdown input render differently:

<div class="green">
   Hello
</div>
<div class ="green">
   Hello
</div>
<div class= "green">
   Hello
</div>
<div class = "green">
   Hello
</div>

if this is parsed using regex, would it be possible to add something like:

/ *= */

if yes, I'd be glad to help

@Jonarod
Copy link
Contributor Author

Jonarod commented Mar 10, 2018

Got into the code and found these bits:

const ATTR_EXTRACTOR_R = /([-A-Z0-9_:]+)(?:\s*=\s*(?:(?:"((?:\\.|[^"])*)")|(?:'((?:\\.|[^'])*)')|(?:\{((?:\\.|{[^}]*?}|[^}])*)\})))?/gi;

and

function attrStringToMap (str) {
        const attributes = str.match(ATTR_EXTRACTOR_R);

        return attributes ? attributes.reduce(function (map, raw, index) {
            const delimiterIdx = raw.indexOf('=');

            if (delimiterIdx !== -1) {
                const key = normalizeAttributeKey(raw.slice(0, delimiterIdx));
                const value = unquote(raw.slice(delimiterIdx + 1));
...

Should be where the planets align together.
Now, I could suggest another approach to prevent parsing attributes twice. In fact, the regex oddly catches relevant groups into one single part, then it is parsed again using indexOf('='), right ?
Couldn't we leverage regex groups to naturally handle attributes key/value split for us ? something like:

/([-A-Z0-9_:]+)\s*=\s*(?:"([^"]+)"|'([^']+)'|\{([^{]+)\})/gi

where 4 groups can be extracted: attribute key, double quoted value, single quoted value and curly brackets enclosed value. Then unquote would not even be needed.

What do you think ?

@Jonarod
Copy link
Contributor Author

Jonarod commented Mar 10, 2018

Submitted PR in a very lightweight fashion (by just trimming down spaces before parsing) + corresponding tests :)

@quantizor
Copy link
Owner

Released as 6.5.1, thanks for your contribution!

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

No branches or pull requests

2 participants