Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support displayName on anonymous components #3192

Merged
merged 10 commits into from
Dec 29, 2021
Merged

Conversation

urugator
Copy link
Collaborator

@urugator urugator commented Nov 24, 2021

partial fix for #2721

Background:

For whatever reason, in react 17, getComponentName returns the name of the memoized component, rather than the name of the wrapper itself:
https://github.com/facebook/react/blob/12adaffef7105e2714f82651ea51936c563fe15c/packages/shared/getComponentName.js#L88
So how does displayName work on memoized component? Memoized component defines displayName as a getter/setter pair, where setter in addition to setting "own" name for the wrapper, also modifies the name of the original component:

const Original = function() {};
const Memoized = memo(Original);
Memoized.displayName = 'memoized';
console.log(Original.displayName === Memoized.displayName); // true

However it does it only if the original component does not have a name yet:

const Original = function() {};
Original.displayName = 'original'
const Memoized = memo(Original);
Memoized.displayName = 'memoized';
console.log(Original.displayName === Memoized.displayName); // false

In this case the Memoized.displayName is practically useless as it's ignored by devtools (you will see original in component tree).

Problem:

Setting displayName (by us) on either original or memoized component prevents customizing displayName by user.

Solution:

Since setting displayName is mostly relevant for anonymous functions, don't set displayName on these (we would set them to empty string anyway).
Additionally, don't hoist displayName as it would redefine memo's getter/setter.

Consequences

Component returned from observer does not have it's own displayName, but the wrapperComponent (available as .type) does, which should be sufficient.

@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2021

🦋 Changeset detected

Latest commit: 8191ca0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mobx-react-lite Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@urugator
Copy link
Collaborator Author

@LandonSchropp added console.log("PR3192") at the beginning of observer, in case you wanna give it another try.
Dunno if necessary, but don't forget to remove the original deps, before linking your locals versions.

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

Successfully merging this pull request may close these issues.

1 participant