Add option to preserve whitespace for certain elements#154
Add option to preserve whitespace for certain elements#154sventschui wants to merge 3 commits intomainfrom
Conversation
Add an option to preserve whitespace/indentation for certain elements. Defaults to `['pre']` Signed-off-by: Sven Tschui <sventschui@gmail.com>
sventschui
force-pushed
the
bugfix/pre-indent
branch
from
bf0b060 to
82daf0f
Compare
src/index.js
Outdated
| rendered += (i > 0 && pretty ? '\n' : '') + renderToString(children[i], context, opts, opts.shallowHighOrder!==false, isSvgMode, selectValue); | ||
| } | ||
| return rendered; | ||
| return pretty ? indent(rendered, indentChar) : rendered; |
There was a problem hiding this comment.
Had to move the call to indent here to not indent HTML content but do indent Fragment children. I'm not 100% sure this doesn't break other things. I'll try to add some more tests.
EDIT yes indeed it breaks text indentation :(
src/index.js
Outdated
| * @param {Boolean} [options.shallow=false] If `true`, renders nested Components as HTML elements (`<Foo a="b" />`). | ||
| * @param {Boolean} [options.xml=false] If `true`, uses self-closing tags for elements without children. | ||
| * @param {Boolean} [options.pretty=false] If `true`, adds whitespace for readability | ||
| * @param {Boolean} [options.preserveWhitespace=['pre']] Array of HTML tags for which to preserve indentation. |
There was a problem hiding this comment.
Nit: type should be string array here 👍
|
It seems the change will break indentation of text nodes. Honestly I'm not sure we can actually change text indent in a safe way. I'd therefore suggest to not indent html (dangerouslySetInnerHtml, string children, etc.) in pretty mode |
…etInnerHTML is in the game
|
Just to keep you posted: I'll add the discussion with @marvinhagemeister on slack here publicly: Marvin mentioned the issue is that we are indenting after rendering instead of during rendering. While trying to change that I ran into issues on how to determine whether we need to wrap/indent text content/children. |
|
This is unfortunately a pretty big issue for Any updates? |
|
@natemoo-re Are you using the |
|
@developit I was at the time! We've since moved to formatting the output as a separate step for other reasons. |
|
@natemoo-re ah okay, good. I'd like to remove this from the default render to string implementation. |
4 participants
Add an option to preserve whitespace/indentation for certain elements. Defaults to
['pre']Caution I'm not 100% sure this doesn't break pretty printing, tests seem fine though.
Fixes #143