Rosey / markdown-draft-js

A tool to convert content created in DraftJS to markdown and vice versa.
https://rosey.github.io/markdown-draft-js/
MIT License
318 stars 70 forks source link

markdownToDraft adds newline to code blocks #19

Closed joepio closed 7 years ago

joepio commented 7 years ago

Hi, thanks again for this awesome library!

When I try to convert an MD code block to Draft, a newline is added to the bottom of the code block. For my application, this means that each time a user switches from Draft editing to MD editing, a newline is added to all his code blocks.

  const initialString = '```\ncountTheNewLines\n```';
  const contentObject = Draft.EditorState.createWithContent(Draft.convertFromRaw(markdownToDraft(initialString)));
  const newString = draftToMarkdown(
    convertToRaw(contentObject.getCurrentContent())
  );
  console.log(newString); // '```\ncountTheNewLines\n\n```'
joepio commented 7 years ago

On a related note, perhaps adding tests for idempotency would be a good idea.

// test/idempotency.spec.js
import { markdownToDraft, draftToMarkdown } from '../src/index';
import { convertFromRaw, convertToRaw, EditorState } from 'draft-js';

describe('idempotency', function () {
  it('does not change code blocks', function () {

    var initialString = '```\ncountTheNewLines\n```';
    var contentObject = EditorState.createWithContent(convertFromRaw(markdownToDraft(initialString)));
    var newString = draftToMarkdown(
      convertToRaw(contentObject.getCurrentContent())
    );

    expect(newString).toEqual(initialString);
  });
});
Rosey commented 7 years ago

Interesting, and frustrating!

Just looking into this a bit, I think the additional newline has to do with the library I'm using to parse markdown, remarkable (https://github.com/jonschlinkert/remarkable) - When I look at the tokens it generates, there is an additional newline there too. It would probably be possible to write a work-around knowing that it does this but I am sad about workarounds based on magic knowledge of a dependency's oddities. Not sure why remarkable does it, there may be a good reason I'm not thinking of. Will have to look into a bit more.

W/R/T idempotency, I think this is a good idea. I think there are some cases though where it would have to fail, for example, this is valid in draftjs: _I am italic content with a space included at the end _ but markdown will struggle with this so we need to convert it to _I am italic content with a space after the closing italic symbol instead of before_ The end result should basically look the same to a human, but behind the scenes it has to be a bit different.

That said, if we can accept some of those limitations and consider them in the tests, I do like the idea of back-and-forth conversion tests where we can.

Rosey commented 7 years ago

PR to fix here: https://github.com/Rosey/markdown-draft-js/pull/26 🙂

Rosey commented 7 years ago

Should be out now in 0.4.1!