Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

inject HOC display name #466

Closed
SimeonC opened this issue May 2, 2018 · 8 comments
Closed

inject HOC display name #466

SimeonC opened this issue May 2, 2018 · 8 comments
Labels
has PR Has PR, so will be fixed soon ready for volunteer

Comments

@SimeonC
Copy link
Contributor

SimeonC commented May 2, 2018

Before running around creating PR's everywhere I thought I'd ask why the inject HOC has a non-standard display name wrapper?

mobx-react/src/inject.js

Lines 37 to 43 in ab90ae6

let displayName =
"inject-" +
(component.displayName ||
component.name ||
(component.constructor && component.constructor.name) ||
"Unknown")
if (injectNames) displayName += "-with-" + injectNames

Currently this means when we debug we get a wrapper like inject-Component or inject-Component-with-storeA. This is inconsistent with the rest of the HOC components like observer(Component).

Why isn't inject using displayName like inject(Component) or inject-with-storeA(Component)?

@mweststrate
Copy link
Member

wasn't aware there was a standard for it :). Feel free to PR

@SimeonC
Copy link
Contributor Author

SimeonC commented May 7, 2018

Yea, I just found out about it when using a dev library that was parsing it this way.
React Docs Reference: https://reactjs.org/docs/higher-order-components.html#convention-wrap-the-display-name-for-easy-debugging
I'll see if I can get a PR up.

@ItamarShDev
Copy link
Member

is it up for grabs, or @SimeonC is still on it?

@SimeonC
Copy link
Contributor Author

SimeonC commented Oct 30, 2018 via email

@ItamarShDev
Copy link
Member

And isn't there any way to add it as a non breaking change?

I'm asking to learn, and not to lecture. I am new to the library

@SimeonC
Copy link
Contributor Author

SimeonC commented Oct 30, 2018

Not really. Though in this case "Breaking Change" shouldn't affect running code but it will affect any dev tools that rely on it. If you were to look at the Display Name (normally in React dev tools) currently you would see something like the following examples.

Before Change (Current)

<inject>
  <Component>
...

With this change it changes to;

<inject(Component)>
  <Component>
...

Or something like that (I'm a little fuzzy on the details - it has been 6 months).

@ItamarShDev
Copy link
Member

should be added the label Breaking change?

@mweststrate
Copy link
Member

mweststrate commented Feb 19, 2019 via email

@mweststrate mweststrate added the has PR Has PR, so will be fixed soon label Mar 14, 2019
@github-actions github-actions bot mentioned this issue Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR Has PR, so will be fixed soon ready for volunteer
Projects
None yet
Development

No branches or pull requests

3 participants