React Native Render HTML: whitespace collapsing (2d shot)

[jsamr]

Context

@AndrewGable This is a follow-up on #851. I've upgraded react-native-render-html to version 6.0.0-alpha.10, which fixes the Regex instantiation error faced in chrome browsers. Just to quote to commit:

This version offers a workaround regarding webpack "Terser" (minifier)
plugin replacing unicode escape sequences in strings with unicode
characters, which in turns results in SyntaxError during Regex
instantiation in chrome browsers, see #876 (could not reproduce in
Firefox). Instead, unicode escape sequences are used directly in a
regex literal, which are not subject to such minifier optimization.

I have tested the build locally, and the error indeed disappeared! To confirm my initial assumptions, I also tested the build with the alpha-8, and applied the below patch to webpack.prod.js. Indeed, the issue also disappeared!

diff --git a/config/webpack/webpack.prod.js b/config/webpack/webpack.prod.js
index 3a1604c4..0c07a62a 100644
--- a/config/webpack/webpack.prod.js
+++ b/config/webpack/webpack.prod.js
@@ -2,9 +2,12 @@ const path = require('path');
 const webpack = require('webpack');
 const {merge} = require('webpack-merge');
 const dotenv = require('dotenv');
+const TerserPlugin = require('terser-webpack-plugin');
 const common = require('./webpack.common.js');
 
-const env = dotenv.config({path: path.resolve(__dirname, '../../.env.production')}).parsed;
+const env = dotenv.config({
+    path: path.resolve(__dirname, '../../.env.production'),
+}).parsed;
 
 module.exports = merge(common, {
     mode: 'production',
@@ -12,6 +15,28 @@ module.exports = merge(common, {
     plugins: [
         new webpack.DefinePlugin({
             __REACT_WEB_CONFIG__: JSON.stringify(env),
-        })
+
+            // React Native JavaScript environment requires the global __DEV__ variable to be accessible.
+            // react-native-render-html uses variable to log exclusively during development.
+            // See https://reactnative.dev/docs/javascript-environment
+            __DEV__: false,
+        }),
     ],
+    optimization: {
+        minimizer: [
+            new TerserPlugin({
+                cache: true,
+                parallel: true,
+                sourceMap: true,
+                terserOptions: {
+                    output: {
+
+                        // When this option is set to false, the terser plugin will replace escape sequences in string (\uxxxx)
+                        // with unicode characters. Unfortunately, this will break Regex instantiation on chrome.
+                        ascii_only: true,
+                    },
+                },
+            }),
+        ],
+    },
 });

Summary

• Upgrade react-native-render-html to version 6.0.0-alpha.10
• Refactored src/styles/StyleSheet (styles are passed to custom renderers, thus there is no need to define distinct styles for code and pre tags)
• Moved HTML rendering logic from src/pages/home/report/ReportActionItemFragment.js to the RenderHTML component, adapted to RNRH v6
• Refactored InlineCodeBlock and adapted its logic to the new API
• Refactored propTypes for AnchorForCommentsOnly
• Remove react-native-render-html from files transpiled by babel-loader (not required anymore, the library already transpiles JSX)
• Added __DEV__ global variable in webpack environment, required by react-native-render-html, see https://reactnative.dev/docs/javascript-environment

Notes

• I disabled inline CSS parsing with the new enableCSSInlineProcessing prop. If you think you might need this feature, I can enable it.
• The new release requires consumers to register available fonts in the app. Use systemFonts for that purpose (I've already registered all the fonts declared in src/styles/fontFamily).
• There is no official documentation for the new release yet. However, being written in Typescript, you can benefit from intellisense and inspect declaration files easily. Also, you can take a look at snippets from the "Foundry Playground" demo, especially on how custom renderers work. All the steps are detailed here: [6.x] 🚀 Welcoming the Foundry release! meliorence/react-native-render-html#430

Suggestions

• I noticed you use a lot the Dimension API. It is known for being unreliable, (see Make a decision about the Dimensions module react-native-community/discussions-and-proposals#291). I recommend useWindowDimensions hook instead!
• .eslintrc.js is in config folder, which is non standard. Would be best if the linter config could be picked automatically by VSCode, very handy for format action!
• There is a plugin for estlint + prettier. It is efficient to avoid conflicting requirements.

Future work

• I didn't refactor the image component since it works as it is. However, I recommend to reuse the internal renderer for images, which handles scaling very well. For that purpose, I initially thought you could use DefaultTagRenderer prop in the renderer component (he would require that you pass down all the props). But I realize that you tamper with the URL for credentials. Instead, you would need to reuse the internal HTMLImageElement component. I haven't exported that yet in the new API, but I will do so soon (see Use internal default renderers logic in custom renderers meliorence/react-native-render-html#424). I could work on a new PR as part of a distinct service if you are interested.
• I will release an important optimization from which the chat app could really benefit. That is currently, for every RenderHTML instance, a new instance of TRenderEngine is created, with a big tree of objects representing the underlying models for HTML tags and CSS properties, which in turns depends on configuration (custom renderers, styles ... Etc). I am planning to provide two additional React components:
  • TRenderEngineProvider, which will take config props and create a React context providing one engine instance;
  • RenderHTMLFragment, which will consume the engine and build the react tree from html / uri.

Considering the linear cost of re-instantiating an engine for each message in the thread, I truly believe it could make a very sensible addition.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/143758

Tests

Open a chat thread and make sure:

• Pre blocks are correctly rendered
• Inline code tags are correctly rendered
• Images thumbnail and modal work
• Anchors are correctly rendered

[AndrewGable]

Nice fix! Can you add on your previous PRs commits to this as we reverted the merge from Expensify:master, so we will have to re-add that code.

[jsamr]

@AndrewGable I just did a git rebase master prior to pushing and somehow, it didn't apply my commits on top of your revert, so I lost the commits! I just "reverted the revert", as it was unfortunately the only option left... Not ideal in terms of commit history, but I guess it's not a big deal either. Should be fine now!

[AndrewGable]

Looks great! Only one pesky merge conflict left to fix.

[jsamr]

@AndrewGable I'm looking into it! Would you think it's fair if I add one extra workday for all this PR work?

[AndrewGable]

Yes, that's fine 👍

[jsamr]

@AndrewGable OK, I successfully rebased on master (I had to port changes to the image renderer from ReportActionItemFragment.js to RenderHTML component). I tested on desktop and everything looked good. I also applied the linter. Hope everything will be fine this time 🙏

[AndrewGable]

Deployed to web with out a hitch! 👍 Thank you!

[jsamr]

@AndrewGable Fantastic! You'll just need to confirm the ending of the mission on the malt.fr platform and the payment will be released on my end 🙂