ampproject / amp-wp

Enable AMP on your WordPress site, the WordPress way.
https://wordpress.org/plugins/amp/
GNU General Public License v2.0
1.79k stars 381 forks source link

Replace `dangerouslySetInnerHTML` with `createInterpolateElement` in React apps #6759

Closed delawski closed 2 years ago

delawski commented 2 years ago

Feature Description

In React components we are often using a combination of dangerouslySetInnerHTML (or <RawHTML>) with one of the i18n functions like __() for rendering translated strings containing HTML elements like:

<p
  dangerouslySetInnerHTML={ {
    __html: __( 'This is <b>bold</b>.', 'amp' ),
  } }
/>

This approach has some limitations. One is that only simple HTML elements can be interpolated this way. E.g. it's not possible to interpolate a React component into a translated string:

- __( 'Go to <ExternalLink href="https://example.com/">example.com</ExternalLink>', 'amp' );
+ __( 'Go to <a href="https://example.com/" target="_blank" rel="noopener noreferrer">example.com</a>', 'amp' );

The second issue is that using dangerouslySetInnerHTML along with translated strings has some security concerns.

However, since WordPress 5.5 there's a solution available at our disposal: createInterpolateElement.

Here's how the same example would look like with createInterpolateElement:

createInterpolateElement(
  __( 'Go to <a>example.com</a>', 'amp' ),
  {
    a: <ExternalLink href="https://example.com/" />,
  },
);

We should replace all occurrences of dangerouslySetInnerHTML or <RawHTML> with createInterpolateElement.

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

pooja-muchandikar commented 2 years ago

Performed smoke testing on the plugin and the features are working as expected ✅ No blockers found so far.