Differences between react mode and aliases + @tangible/window

[nicolas-jaussaud]

Hi @eliot-akira!

I have a question regarding the "React mode" argument of the roller (as long as it's not really an issue, I thought I could use this "discussions" feature for it)

I'm currently testing a library with the roller (lexical), with a setup similar to this one:

module.exports = {
  build: [
    {
      src: 'index.jsx',
      dest: 'build.js',
      alias: {
        'react': '@tangible/window/wp-element',
        'react-dom': '@tangible/window/wp-element'
      }
    }
  ]
}

index.jsx being something like this:

import { render } from 'react'
import { LexicalComposer } from '@lexical/react/LexicalComposer'

const div = document.createElement('div')
document.querySelector('body').appendChild(div)

render(
    <div>
        <LexicalComposer initialConfig={{
            namespace: 'tangible'
        }}>
        </LexicalComposer>
    </div>
, div)

It works well, but before I didn't use @tangible/window and had this config instead:

module.exports = {
  build: [
    {
      src: 'index.jsx',
      dest: 'build.js',
      react: 'wp'
    }
  ]
}

Which wasn't working and gave me the following error:

Warning: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.

For what I understood, it was because a different version of React was imported by lexical, instead of wp-element

I'm not sure to understand why it works differently when using an alias with @tangible/window, and until now I was assuming that it was the same thing than using react: 'wp'

I was wondering what the differences between the two approaches are, and which method should be used in which cases

[eliot-akira]

Hi @nicolas-jaussaud, thanks for the question.

You're right that react: 'wp' is supposed to work the same as @tangible/window. It's meant to be a simpler solution that doesn't require an additional NPM package.

The @tangible/window package was a workaround for aliasing imports to global variables like wp.element, Tangible.Preact, and jQuery.

The new solution is to use rollup-plugin-external-globals. In the task config, it's the importToGlobal option, which gets passed to this plugin.

Internally, React mode wp sets the importToGlobal option for react and react-dom to reference wp.element.

https://github.com/TangibleInc/tangible-roller/blob/37067684eb1a63da231ffada567e09a91e1d7259/task/js.js#L130-L136

From your description, it sounds like this is not being applied to the lexical package in node_modules.

..Oh, maybe you're importing the compiled/built library files of lexical, which uses the require method (called CommonJS) to import React. And it's possible this Rollup plugin only works with export/import (called ES Modules). (For explanation of these import syntaxes: CommonJS vs. ES modules in Node.js.) Mmm, but when I implemented the React modes, I'm pretty sure I checked that it works with compiled packages in node_modules..

So I would say this is a bug in the React mode wp. I'll try to recreate the issue locally and see if I can find a solution.

(Ah, cool - I can "Create issue from discussion" in the GitHub sidebar menu. Issue #3 )

[eliot-akira]

Whew, it took a couple hours of digging deep into the Rollup plugins, but I believe I solved it in commit d4a508d.

The cause of the issue was that for require statements, the CommonJS plugin transforms the module names into absolute paths, for example, react into .../node_modules/react/cjs/react.production.min.js. (This only happens if regular react is installed in node_modules. Otherwise, the module name is left as is.) That's why the External Globals plugin didn't recognize it in order to transform it into a global variable, in this case wp.element.

The solution was to pass to the latter plugin a function that matches the module name as is (for import statements) and as absolute paths (for require statements).

It's been published as version 1.4.4. Could you run npm update in your local project and see if the React mode wp works now?

[nicolas-jaussaud]

Thank you for the quick update!

I tested again and while components imported from the lexical library are now using wp.element, it seems that there is an issue in the way it's imported

Here is the error I got after updating and building a new file using the react: 'wp' option (I tested on the same index.jsx than in my first message):

Uncaught TypeError: window.wp.element.__require is not a function

In the built file, I have indeed multiple occurrences of something like:

var react = wp.element.__require();

If I manually replace every occurence of wp.element.__require() by wp.element, the script works as expected

[eliot-akira]

Ah, actually - I think I found a much better solution.

In commit 123e409, I added a new Rollup option called external so that any imported module aliased to global variable will not be transformed by the CommonJS plugin. And I removed the whole workaround with absolute module path and require function wrapper.

Please try Roller 1.4.6. Hopefully that works, because this would be a clean and satisfying solution. If not, I'll restore the workaround as a fallback.

[nicolas-jaussaud]

Hi!

Thank you for your help, updating to 1.4.6 indeed fixes the issue!

Unfortunately, it seems to cause a different error with another import from another library, which was working previously (react-transition-group)

In the files from their npm module, react is required like this:

var _react = _interopRequireDefault(require("react"));

function _interopRequireDefault(obj) { 
    return obj && obj.__esModule ? obj : { default: obj }; 
}

function (_React$Component) {
   function ComponentName(props, context) {
    // ...
   }
}(_react.default.Component);

In our built file, the _react variable becomes something like this:

var _react = _interopRequireDefault(_global_wp_element$1);

The issue is that while _global_wp_element$1.__esModule is true, _global_wp_element$1.default does not exist

But as long as the _interopRequireDefault$4 relies on the __esModule attribute to set or not a default value, it will not be set and will throws an error when reaching the _react$3.default.Component part:

Uncaught TypeError: _react$3.default is undefined

I guess somehow the roller was setting this default value before?

(I attached a small wordpress plugin that replicates the issue)
tangible-test.zip

[eliot-akira]

Oh my, what a rabbit hole this is turning out to be, haha.

The __esModule wrapper looks like something that the CommonJS plugin should be processing. OK, so I'll revert to the workaround in the previous version of Roller, which lets the aliased modules go through CommonJS (and then overwrite them as globals).

I appreciate the example zip, that will help me confirm when I actually get it working.

---

The real underlying cause of issue is that rollup-plugin-external-globals is not working well with @rollup/plugin-commonjs. (And also that Rollup's option globals is not working as expected.)

It's almost too much for me to try to solve their compatibility issue - these JavaScript transpilers are crazy complex. When I have more time, I will see if I can prepare an issue report and hopefully the plugin author can solve it properly.

But in the meantime, I think I'm close to finding an acceptable temporary solution.

[eliot-akira]

Commit 055be89 should do it. Please try Roller version 1.4.7 by running npm update.

[nicolas-jaussaud]

Thanks a lot!