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

Spaces between words and inline elements are lost #135

Closed
davidtheclark opened this issue Aug 5, 2017 · 4 comments · Fixed by #205
Closed

Spaces between words and inline elements are lost #135

davidtheclark opened this issue Aug 5, 2017 · 4 comments · Fixed by #205

Comments

@davidtheclark
Copy link

If I start with this JSX:

<div>
  foo <strong>bar</strong> baz
</div>

That renders to React element that with children like this:

[ 
  'foo ',
  { type: 'strong', props: { children: [Object] }, .. },
  ' baz'
]

If I then pass that element through this module, I come out with this JSX — replacing some whitespaces with * to clarify:

<div>
  foo*
  <strong>
    bar
  </strong>
  *baz
</div>

The problem is that with the reformatting, those whitespace are actually lost. End-of-line and beginning-of-line whitespace is not meaningful, so the output HTML is actually this:

<div>
  foo<strong>bar</strong>baz
</div>

If introducing all the line breaks is the preference here, I think one fix would be to add spaces as strings, e.g.

<div>
  foo{' '}
  <strong>
    bar
  </strong>
  {' '}baz
</div>
@digital-flowers
Copy link

digital-flowers commented Sep 8, 2017

i think you can use this alternative function just pass inline:true to the options

import formatTreeNode from 'react-element-to-jsx-string/dist/formatter/formatTreeNode';
import parseReactElement from 'react-element-to-jsx-string/dist/parser/parseReactElement';

const jsxToString = (element, {
  filterProps = [],
  showDefaultProps = true,
  showFunctions = false,
  tabStop = 2,
  useBooleanShorthandSyntax = true,
  sortProps = true,
  inline = false,
  ...rest
}) => {
  const options = {
    filterProps,
    showDefaultProps,
    showFunctions,
    tabStop: inline ? 0 : tabStop,
    useBooleanShorthandSyntax,
    sortProps,
    ...rest
  };
  const str = formatTreeNode(parseReactElement(element, options), inline, 0, options);
  return inline ? str.replace(/\n/g, "").replace(/\s+/g, " ") : str;
};

Something like

jsxToString(element, {inline: true}));

@armandabric
Copy link
Collaborator

After some thought on this issue. I've found that the prettier project propose the same solution as you suggest @davidtheclark for the trailing space at the end of the line.

We could give a try on this solution. I'm little scare that the resulting string will be more difficult to read. It could be an issue for some use case.

armandabric added a commit to armandabric/react-element-to-jsx-string that referenced this issue Oct 8, 2017
Closes algolia#135

BREAKING CHANGE: Trailling are now preserved. In some rare case, `react-element-to-jsx-string` failed to respect the JSX specs for the trailing space. Event is the space were in the final output. There were silentrly ignored by JSX parser. This commit fix this bug by protecting the trailing space in the output.

If we take the JSX:
```jsx
<div>
  foo <strong>bar</strong> baz
</div>
```

Before it was converted to (the trailing space are replace by `*` for the readability):
```html
<div>
  foo*
  <strong>
    bar
  </strong>
  *baz
</div>
```

Now there are preserved:
```html
<div>
  foo{' '}
  <strong>
    bar
  </strong>
  {' '}baz
</div>
```
armandabric added a commit to armandabric/react-element-to-jsx-string that referenced this issue Oct 8, 2017
Closes algolia#135

BREAKING CHANGE: Trailling are now preserved. In some rare case, `react-element-to-jsx-string` failed to respect the JSX specs for the trailing space. Event is the space were in the final output. There were silentrly ignored by JSX parser. This commit fix this bug by protecting the trailing space in the output.

If we take the JSX:
```jsx
<div>
  foo <strong>bar</strong> baz
</div>
```

Before it was converted to (the trailing space are replace by `*` for the readability):
```html
<div>
  foo*
  <strong>
    bar
  </strong>
  *baz
</div>
```

Now there are preserved:
```html
<div>
  foo{' '}
  <strong>
    bar
  </strong>
  {' '}baz
</div>
```
@armandabric
Copy link
Collaborator

Here a PR that propose a solution to fix the issue #205

armandabric added a commit to armandabric/react-element-to-jsx-string that referenced this issue Oct 8, 2017
Closes algolia#135

BREAKING CHANGE: Trailing are now preserved. In some rare case, `react-element-to-jsx-string` failed to respect the JSX specs for the trailing space. Event is the space were in the final output. There were silentrly ignored by JSX parser. This commit fix this bug by protecting the trailing space in the output.

If we take the JSX:
```jsx
<div>
  foo <strong>bar</strong> baz
</div>
```

Before it was converted to (the trailing space are replace by `*` for the readability):
```html
<div>
  foo*
  <strong>
    bar
  </strong>
  *baz
</div>
```

Now there are preserved:
```html
<div>
  foo{' '}
  <strong>
    bar
  </strong>
  {' '}baz
</div>
```
armandabric added a commit that referenced this issue Oct 9, 2017
Closes #135

BREAKING CHANGE: Trailing are now preserved. In some rare case, `react-element-to-jsx-string` failed to respect the JSX specs for the trailing space. Event is the space were in the final output. There were silentrly ignored by JSX parser. This commit fix this bug by protecting the trailing space in the output.

If we take the JSX:
```jsx
<div>
  foo <strong>bar</strong> baz
</div>
```

Before it was converted to (the trailing space are replace by `*` for the readability):
```html
<div>
  foo*
  <strong>
    bar
  </strong>
  *baz
</div>
```

Now there are preserved:
```html
<div>
  foo{' '}
  <strong>
    bar
  </strong>
  {' '}baz
</div>
```
@davidtheclark
Copy link
Author

I'm little scare that the resulting string will be more difficult to read.

I agree that it makes the output more difficult to read —— but at least it works as expected.

Thanks for looking into this 👍

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

Successfully merging a pull request may close this issue.

3 participants