Fix for passing classname to the DOM in WebComponents created with Html2React

[michalczaplinski]

What:

Fix the issue where adding style on a custom component like amp-img causes passing of className property to the DOM.

Why:

To fix #742

How:
I think we can change it by updating the parse function in @frontity/html2react to check if the current node is a custom component.

Tasks:

• Code
• TSDocs
• TypeScript
• Unit tests
• Add a changeset (with link to its Feature Discussion if it exists)

Unrelated Tasks

• End to end tests
• TypeScript tests
• Update starter themes
• Update other packages
• Update community discussions

Additional Comments

[changeset-bot]

🦋 Changeset detected

Latest commit: 41a3fc4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@frontity/html2react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[codecov]

Codecov Report

Merging #741 (41a3fc4) into dev (a46c71a) will decrease coverage by 0.10%.
The diff coverage is 64.29%.

Impacted Files	Coverage Δ
packages/html2react/src/libraries/component.tsx	78.83% <64.29%> (-7.38%)	⬇️

[michalczaplinski]

@DAreRodz I've realized that using @emotion/css is not going to work correctly with SSR so I'm removing you from reviewers until I figure out a better solution.

[luisherranz]

I've proposed another approach here: #742 (comment)

[michalczaplinski]

I've added https://emotion.sh/docs/@emotion/jest to improve jest snapshots.

[michalczaplinski]

I've posted it in the #742 but here again for completeness.

https://www.loom.com/share/fd3e727cdb314fe1b1aa09c4807043ce

[luisherranz]

You are absolutely right.

What about using a <Wrapper>, like Mateusz explained here: emotion-js/emotion#1071 (comment)?

I have done a quick test and I think it can work: https://www.loom.com/share/3b38039890a2499297959026a2ef9b9e

This is the code of the Wrapper:

const Wrapper = ({ Component, children, className, ...props }) => {
  return (
    <Component {...props} class={className}>
      {children}
    </Component>
  );
};

if (
  typeof node.component === "string" &&
  isCustomComponent(node.component, node.props)
) {
  return (
    <Wrapper
      key={index}
      css={node.props.css}
      Component={node.component}
      {...node.props}
    >
      {node.children ? handleNodes({ nodes: node.children, ...payload }) : null}
    </Wrapper>
  );
}

Also, could you please add more props and children to the tests? That way we can make sure that either solution you finally choose (Wrapper or ClassNames) will correctly pass down any props and children to the custom component.

I'm going to approve this now so you can merge it once it's done and continue your work with the AMP processors 🙂👍

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[michalczaplinski]

OK, awesome. Thanks for the tip about the Wrapper component @luisherranz. You're right it makes the implementation simpler. 👍

I've added some extra tests to check for passing children, passing other props and handling nested custom components like you suggested.

I've also realized that we have another small bug in the parse() function: If an element in the content has a className attribute, it will be passed to the DOM as class and also overwrite any class attribute on that element. Raised a bug: #747

Will merge this now 🙂

[luisherranz]

Awesome. Great work Michal! 👏😄