[feature] replace supports return false and node object

[lizheming]

What is the motivation for this pull request?

Add feature to replace method.

What is the current behavior?

replace method only support return React Element now. It's difficult if we want remove or modify element. I can return modified React Element, but it's not easy for some node which has children.

What is the new behavior?

If replace method return false boolean we can remove this node, and if replace method return a parser node, we can update html AST node.

Checklist:

• Tests
• Documentation

[remarkablemark]

Hi @lizheming, thanks for taking the time to open this PR!

I understand that the approach to replace or remove elements isn't as straightforward as we'd like, but do the current workarounds not satisfy your requirements?

1. Example of removing an element:

parse('<div class="remove">', {
  replace: ({ attribs }) => {
    if (attribs && attribs.class === 'remove') {
      return <React.Fragment />; // React.createElement(React.Fragment);
    }
  },
});

2. Example of replacing an element:

parse('<div class="replace">', {
  replace: domNode => {
    if (domNode.attribs && domNode.attribs.class === 'replace') {
      domNode.name = 'p';
      return parse.domToReact(domNode);
    }
  },
});

Both examples can be found in the README.

Although I find your additions useful, I'm hesitant in merging them because I'm a big believer of keeping the parser API as simple as possible. What are your thoughts?

[lizheming]

Thank you for your reply. I'm agree with you that API should be simple. Remove action can be done as you said, but it's not easy to replace some node which has children and we want to do replace action recursively. Although we can use parse() again, but the repeat code we should write, that it's not simple for API user.

Before:

const options = { 
  replace(domNode) {
    if (domNode.attribs && domNode.attribs.class === 'replace') {
      const children = parse(domNode.children, options); // it's a repeat logic because module has handle it after.
      return React.createElement('p', {}, children);
    }
  }
};
parse('<div class="replace"><div class="replace"></div></div>', options);

VS.

After:

parse('<div class="replace"><div class="replace"></div></div>', { 
  replace(domNode) {
    if (domNode.attribs && domNode.attribs.class === 'replace') {
      domNode.name = 'p';
      return domNode;
    }
  }
});

[remarkablemark]

Okay, I see what you mean. I do agree that the code gets a bit more complex when performing recursion with replace. However, I still feel that the cons outweigh the pros.

1. Additional overhead is added to the replace API.
   • Users will now need to keep in mind of 2 return types.
   • Type safety is lost as anyone can return a POJO (Plain Old JavaScript Object) that is similar to the node:

   parse('<div class="replace">', {
     replace: domNode => {
       if (domNode.attribs && domNode.attribs.class === 'replace') {
         return {
           type: 'tag',
           name: 'foo',
           attribs: {},
         };
       },
     };
   });

2. The operation is not pure.
   • To modify the node, the node object can be mutated.
   • This may cause unintended side-effects:

   parse('<div class="replace">', {
     replace: domNode => {
       if (domNode.attribs && domNode.attribs.class === 'replace') {
         domNode.next = null;
         return domNode;
       },
     };
   });

3. The code is slightly more DRY (Don't Repeat Yourself), but it didn't shave off that many lines.
   • The only difference is code organization.
   • Options is moved out and <p>{parse(domNode.children, options)}</p> is added.

What do you think about my arguments—do you agree/disagree with them?

[lizheming]

OK, That's just different choice for user, I agree with the second opinion and respect your choice.