Repository: symfony/ux
Pull request #2006 (2024-07-24 19:23:05)

[React] Improve error handling in `resolveReactComponent`
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Issues        | 
| License       | MIT

## WHAT

Improve error handling and readability in `window.resolveReactComponent` function

- Enhance error messages for clarity:
  - Indicate when a module exists but lacks a default export.
  - (still) List available controllers when a specified controller does not exist.

With those changes, when resolving a module with no default export, an error message with a hint will be shown: 

```
React controller "${name}" could not be resolved. Ensure the module exports the controller as default.
```

## WHY

I found that when someone creates a module without a default export then the error message is quite confusing. 
For example:
```js
// NoDefaultExportComponent.jsx
export const SomeRandomName = () => {
    return <div>Hello</div>;
}
```
Whenever there will be an element like this 

```html
<div data-controller="NoDefaultExportComponent">
   ...
</div>
```
An error like below in console appear:

> Error connecting controller
>
> React controller "**NoDefaultExportComponent**" does not exist. Possible values: SomeComponent, AnotherComponent, **NoDefaultExportComponent**

Which is not exactly true. The module `NoDefaultExportComponent` was found but there is no required default export.


teklakct: Alternatively function `importAllReactComponents` may omit every module without default export. Something like this

```diff
    const importAllReactComponents = (r: __WebpackModuleApi.RequireContext) => {
-        r.keys().forEach((key) => (reactControllers[key] = r(key).default));
+        r.keys().forEach((key) => {
+            if (r(key).default !== undefined) {
+                reactControllers[key] = r(key).default;
+            }
+        });
    };
```

However, with these changes, it will be more difficult to determine if a module or default export is missing.
smnandre: Hi @teklakct ! Thanks for this 

Could you rebuild the JS dist files ? (that's the reason of the CI failure)

From the root of the monorepo: `yarn run build`

(you may need to rebase before)
github-actions[bot]: <h1>📊 Packages dist files size difference</h1>

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

<table>
    <thead><tr><th>File</th><th>Before (Size / Gzip)</th><th>After (Size / Gzip)</th></tr></thead>
    <tbody><tr><td colspan="3"><a href="https://github.com/teklakct/ux/tree/feature/resolve_react_error_message/src/React/assets/dist"><b>React</b></a></td></tr><tr>
                <td><a href="https://github.com/teklakct/ux/blob/feature/resolve_react_error_message/src/React/assets/dist/register_controller.js"><code>register_controller.js</code></a></td>
            <td>
                    <code>827 B</code>
                    / <code>398 B</code> 
                </td><td>
                    <code>1.03 kB</code><sup>+28% 📈</sup>
                    / <code>466 B</code><sup>+17% 📈</sup>
                </td></tr></tbody>
</table>

<!-- Sticky Pull Request Comment -->
teklakct: @smnandre sorry for the delay. I must have missed the notification earlier.
smnandre: @teklakct don't be :) I also lost tracking of this PR to be honest! 

Seems good to me !

Patch:
diff --git a/src/React/CHANGELOG.md b/src/React/CHANGELOG.md
index c9c5c066..da9a6a6b 100644
--- a/src/React/CHANGELOG.md
+++ b/src/React/CHANGELOG.md
@@ -1,9 +1,13 @@
 # CHANGELOG
 
+## 2.26.0
+
+-   Improve error handling when resolving a React component
+
 ## 2.21.0
 
--   Add `permanent` option to the `react_component` Twig function, to prevent the 
-    _unmounting_ when the component is deconnected and immediately re-connected. 
+-   Add `permanent` option to the `react_component` Twig function, to prevent the
+    _unmounting_ when the component is deconnected and immediately re-connected
 
 ## 2.13.2
 
diff --git a/src/React/assets/dist/register_controller.js b/src/React/assets/dist/register_controller.js
index 2eef8cd8..d0c38da6 100644
--- a/src/React/assets/dist/register_controller.js
+++ b/src/React/assets/dist/register_controller.js
@@ -10,6 +10,10 @@ function registerReactControllerComponents(context) {
         const component = reactControllers[`./${name}.jsx`] || reactControllers[`./${name}.tsx`];
         if (typeof component === 'undefined') {
             const possibleValues = Object.keys(reactControllers).map((key) => key.replace('./', '').replace('.jsx', '').replace('.tsx', ''));
+            if (possibleValues.includes(name)) {
+                throw new Error(`
+                    React controller "${name}" could not be resolved. Ensure the module exports the controller as a default export.`);
+            }
             throw new Error(`React controller "${name}" does not exist. Possible values: ${possibleValues.join(', ')}`);
         }
         return component;
diff --git a/src/React/assets/src/register_controller.ts b/src/React/assets/src/register_controller.ts
index e9d59afe..8be8923a 100644
--- a/src/React/assets/src/register_controller.ts
+++ b/src/React/assets/src/register_controller.ts
@@ -37,6 +37,12 @@ export function registerReactControllerComponents(context: __WebpackModuleApi.Re
             const possibleValues = Object.keys(reactControllers).map((key) =>
                 key.replace('./', '').replace('.jsx', '').replace('.tsx', '')
             );
+
+            if (possibleValues.includes(name)) {
+                throw new Error(`
+                    React controller "${name}" could not be resolved. Ensure the module exports the controller as a default export.`);
+            }
+
             throw new Error(`React controller "${name}" does not exist. Possible values: ${possibleValues.join(', ')}`);
         }
 
diff --git a/src/React/doc/index.rst b/src/React/doc/index.rst
index 92097ca9..0faa32c4 100644
--- a/src/React/doc/index.rst
+++ b/src/React/doc/index.rst
@@ -81,6 +81,10 @@ For example:
         return <div>Hello {props.fullName}</div>;
     }
 
+.. note::
+
+    Ensure your module exports the controller as the ``export default``. The default export is used when resolving components.
+
 .. code-block:: html+twig
 
     {# templates/home.html.twig #}


Test patch:
diff --git a/src/React/assets/test/register_controller.test.tsx b/src/React/assets/test/register_controller.test.tsx
index 4595a7d9..dcb38082 100644
--- a/src/React/assets/test/register_controller.test.tsx
+++ b/src/React/assets/test/register_controller.test.tsx
@@ -17,6 +17,7 @@ const createFakeFixturesContext = (): RequireContext => {
     const files: any = {
         './MyJsxComponent.jsx': { default: MyJsxComponent },
         './MyTsxComponent.tsx': { default: MyTsxComponent },
+        './NoDefaultExportComponent.jsx': { default: undefined },
     };
 
     const context = (id: string): any => files[id];
@@ -45,4 +46,13 @@ describe('registerReactControllerComponents', () => {
             'React controller "MyABCComponent" does not exist. Possible values: MyJsxComponent, MyTsxComponent'
         );
     });
+
+    it('throws when no default export found in imported module', () => {
+        registerReactControllerComponents(createFakeFixturesContext());
+        const resolveComponent = (window as any).resolveReactComponent;
+
+        expect(() => resolveComponent('NoDefaultExportComponent')).toThrow(
+            'React controller "NoDefaultExportComponent" could not be resolved. Ensure the module exports the controller as a default export.'
+        );
+    });
 });