Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Feature request: Allow to disable displayName wrapping globally #559

Open
finom opened this issue Nov 11, 2017 · 11 comments
Open

Feature request: Allow to disable displayName wrapping globally #559

finom opened this issue Nov 11, 2017 · 11 comments

Comments

@finom
Copy link

finom commented Nov 11, 2017

I widely use Recompose at all projects I do. And it's not useful to see displayNames like pure(withHandlers(withProps(withPropsOnChange(ComponentName)))) at React dev tools and I would like to see just ComponentName instead because wrapped names don't make sense for me (and I guess for 99% developers) at all. I would like to disable name wrapping globally. Is that something you'd implement?

Thank you for the great library!

@istarkov
Copy link
Contributor

istarkov commented Nov 11, 2017

So we will have a lot of comonents with same names, as every HOC is a component itself.
I also don't like current names, mb it will be better to reverse and somehow simplify names like
ComponentName_wPOC_wP_wH_p

@timkindberg
Copy link
Contributor

Maybe the react devtools could handle HOCs in a special manner. Devs could maybe set a static property or something to let devtools know it is a HOC.

What I’m saying is maybe this is a concern for our tooling and not so much for this library.

@goloveychuk
Copy link

goloveychuk commented Mar 27, 2018

@istarkov @timkindberg
Could we update wrapDisplayName to have

const wrapDisplayName = (BaseComponent, hocName) => {
   if (process.env.RECOMPOSE_SAME_DISPLAY_NAME) {
          return BaseComponent
   }
  return  `${hocName}(${getDisplayName(BaseComponent)})`
}

We need this for react virtual dom serializers (jest snapshots e.g., enzyme). In our tests and storybook we have
<withState(withHandlers(defaultProps(....
It'd be good to have some option to disable this, other than NODE_ENV.
I could submit a pr if you agree with this change.

@istarkov
Copy link
Contributor

Isnt it will cause a lot of same displayNames? When every hoc have the same name as a base? (code above looks strange :-) not sure what you want to return inside if

@istarkov
Copy link
Contributor

Also I wanna say I highly dislike current displayName approach so open to any proposals

@goloveychuk
Copy link

it will. So use case. We have storybook, which displays source code of example.
To conveniently mock some things we have

const enhance = compose(
    withState('value', 'onChange', props => props.value),
    withHandlers({
        onChange: props => value => {
            action('change')(value);
            props.onChange(value);
        },
        onFocus: () => action('focus'),
        onBlur: () => action('blur'),
        onLabelClick: () => action('labelClick'),
        onCut: () => action('cut'),
        onCopy: () => action('copy'),
        onPaste: () => action('paste'),
    })
)

Than compose our component and show in storybook.
And have this <withState ...
So yeah, I know that we will have same names both for original and composed component, but sometimes it doesn't matter (like in this case :))

@goloveychuk
Copy link

goloveychuk commented Mar 27, 2018

not sure what you want to return inside if

original displayName

upd:
yeah, my mistake, should be
return getDisplayName(BaseComponent)

@istarkov
Copy link
Contributor

Could we somehow allow users to redefine wrapDisplayName for example using React Context without a big perf affect at dev and possibly prod mode? ( We can always add something like registerWrapDisplayName which will change the default behaviour but I dont like the idea of changing global exports) so any ideas?

@adamhenson
Copy link

adamhenson commented Mar 27, 2018

I'm seeing issues in our Jest Snapshot testing that seem related to this. In our production build pipeline we run tests with NODE_ENV=production and are seeing failures with the following diffs in our snapshot files.

-         <getContext(HelloWorld)
+         <GetContext

and:

-         <Component
+         <withProps(FooBar)

I'm guessing it has something to do with this issue... more specifically:

if (process.env.NODE_ENV !== 'production') {
return setDisplayName(wrapDisplayName(BaseComponent, 'getContext'))(
GetContext
)
}

And this, etc:

if (process.env.NODE_ENV !== 'production') {
return BaseComponent =>
setDisplayName(wrapDisplayName(BaseComponent, 'withProps'))(
hoc(BaseComponent)
)
}

If anyone can confirm that this behavior is expected I'd appreciate it. Otherwise, just posting here for anyone else who comes across it. I forked this repo and will see if I can confirm as well.

In my opinion it doesn't seems ideal for components to have different names by environment. I imagine it would be an issue affecting anyone using recompose and snapshot testing.


If this is in fact the expected behavior, perhaps we should add a way of opting out with either an option or NODE_ENV (since we're already using it in this case). This would help those of us who use the combination of snapshot testing, recompose, and run the same tests in more than one NODE_ENV.

@adamhenson
Copy link

Also, perhaps my above comment justifies a separate issue (support for snapshot tests). Please let me know and I can open one.

@metreniuk
Copy link

@istarkov I encountered the same problem with long display names at work. We have our own HOCs and we are flattening the display name to the most relevant one.
Something like this:
withState(withHandlers(mapProps(Component))) -> withState(Component).
This way each component in the devtools tree will tell only about its functionality:
image
vs
image
This also is much more pleasant while debugging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants