mobxjs / mobx-react-lite

Lightweight React bindings for MobX based on React 16.8 and Hooks
https://mobx.js.org/react-integration.html
MIT License
2.13k stars 91 forks source link

Retain function name, when using observer wrapper with an arrow function? #242

Closed RosenTomov closed 4 years ago

RosenTomov commented 4 years ago

Currently, I'm using 'ErrorBoundary' class component (implements componentDidCatch etc.) that wraps up the hooks, since they don't support error catching yet.

<ErrorBoundary>
  <App>
</ErrorBoundary>

Is there some tricky way to retain or set the function name, when using observer wrapper with an arrow function?

const App = observer(() => { 
...
throw new Error('test')
...
});

Results in

The above error occurred in the <wrappedComponent> component:
    in wrappedComponent
    in ErrorBoundary

I've tried settings displayName and defaultProps.name with no luck.

The only workaround so far is to convert the arrow function to a named one:

const App = observer(function App() { 
....
});

Which results in:

The above error occurred in the <App> component:
    in App
    in ErrorBoundary
mweststrate commented 4 years ago

Use App.displayname = "App"

Op wo 20 nov. 2019 11:29 schreef RosenTomov notifications@github.com:

Currently, I'm using 'ErrorBoundary' class component (implements componentDidCatch etc.) that wraps up the hooks, since they don't support error catching yet.

Is there some tricky way to retain or set the function name, when using observer wrapper with an arrow function?

const App = observer(() => { ... throw new Error('test') ... });

Results in

The above error occurred in the component: in wrappedComponent in ErrorBoundary

I've tried settings displayName and defaultProps.name with no luck.

The only workaround so far is to convert the arrow function to a named one:

const App = observer(function App() { .... });

Which results in:

The above error occurred in the component: in App in ErrorBoundary

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-react-lite/issues/242?email_source=notifications&email_token=AAN4NBFCDMBC74TTNNTMHFDQUUNTHA5CNFSM4JPRIMT2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H2WHNLA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBGLZTKP7OL2NFYRAODQUUNTHANCNFSM4JPRIMTQ .

RosenTomov commented 4 years ago

It didn't work, I've also tried with "defaultProps.name".

mweststrate commented 4 years ago

sorry, should be displayName I think. It is a standard React feature.

On Wed, Nov 20, 2019 at 12:51 PM RosenTomov notifications@github.com wrote:

It didn't work, I've also tried with "defaultProps.name".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-react-lite/issues/242?email_source=notifications&email_token=AAN4NBEV4WMW4YTXD7Y3AJLQUUXERA5CNFSM4JPRIMT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEER3Z7Y#issuecomment-555990271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBCE7R5WOV5KHB2EZALQUUXERANCNFSM4JPRIMTQ .

RosenTomov commented 4 years ago

Yeah, that's what I mean

App.displayName = 'App';
mweststrate commented 4 years ago

That should work, can you create a reproduction code sandbox?

mweststrate commented 4 years ago

For further details see https://mobx.js.org/best/pitfalls.html#the-display-name-of-react-components-is-not-set

danielkcz commented 4 years ago

We can hopefully solve this better eventually with a macro #203

RosenTomov commented 4 years ago

Sorry for the delay, here's an example https://codesandbox.io/s/mobx-react-lite-example-su99m?fontsize=14&hidenavigation=1&theme=dark

Check the console.

The above error occurred in the <wrappedComponent> component:
    in wrappedComponent (at src/index.js:10)
    in TestWrapper (at src/index.js:9)

even with App.displayName = "App";

And with named function (const App = observer(function App() {)

The above error occurred in the <App> component:
    in App (at src/index.js:10)
    in TestWrapper (at src/index.js:9)

Aside from named functions, using the observer on the export seems to work. export default observer(App); (2 & 4 from https://mobx.js.org/best/pitfalls.html#the-display-name-of-react-components-is-not-set)

danielkcz commented 4 years ago

Sorry for the delay here. I've tried to debug this and I don't think there is anything we can do about it.

Using App.displayName = 'App' doesn't work because the error message is generated by React which looks at App.type.displayName internal object. That also applies to React Devtools.

There are two ways to tackle this correctly and the name will be correctly displayed in both cases.

const App = () => {
  return <div className="App">TEST</div>;
};

export default observer(App);
const App = observer(function App() {
  return <div className="App">TEST</div>;
});

export default App

Closing this for now. Let us know if you have further questions.