-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat: support more element types #617
Changes from 5 commits
d642956
6053e34
3d09871
507e248
6652007
7bb97da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,18 @@ | ||
/* @flow */ | ||
|
||
import React, { type Element as ReactElement, Fragment } from 'react'; | ||
import { | ||
ForwardRef, | ||
isContextConsumer, | ||
isContextProvider, | ||
isForwardRef, | ||
isLazy, | ||
isMemo, | ||
isProfiler, | ||
isStrictMode, | ||
isSuspense, | ||
Memo, | ||
} from 'react-is'; | ||
import type { Options } from './../options'; | ||
import { | ||
createStringTreeNode, | ||
|
@@ -12,12 +24,58 @@ import type { TreeNode } from './../tree'; | |
|
||
const supportFragment = Boolean(Fragment); | ||
|
||
const getReactElementDisplayName = (element: ReactElement<*>): string => | ||
element.type.displayName || | ||
(element.type.name !== '_default' ? element.type.name : null) || // function name | ||
(typeof element.type === 'function' // function without a name, you should provide one | ||
? 'No Display Name' | ||
: element.type); | ||
const getFunctionTypeName = (functionType): string => { | ||
if (!functionType.name || functionType.name === '_default') { | ||
return 'No Display Name'; | ||
} | ||
return functionType.name; | ||
}; | ||
|
||
const getWrappedComponentDisplayName = (Component: *): string => { | ||
switch (true) { | ||
case Boolean(Component.displayName): | ||
return Component.displayName; | ||
case Component.$$typeof === Memo: | ||
return getWrappedComponentDisplayName(Component.type); | ||
case Component.$$typeof === ForwardRef: | ||
return getWrappedComponentDisplayName(Component.render); | ||
default: | ||
return getFunctionTypeName(Component); | ||
} | ||
}; | ||
|
||
// heavily inspired by: | ||
// https://github.com/facebook/react/blob/3746eaf985dd92f8aa5f5658941d07b6b855e9d9/packages/react-devtools-shared/src/backend/renderer.js#L399-L496 | ||
const getReactElementDisplayName = (element: ReactElement<*>): string => { | ||
switch (true) { | ||
case typeof element.type === 'string': | ||
return element.type; | ||
case typeof element.type === 'function': | ||
if (element.type.displayName) { | ||
return element.type.displayName; | ||
} | ||
return getFunctionTypeName(element.type); | ||
case isForwardRef(element): | ||
case isMemo(element): | ||
return getWrappedComponentDisplayName(element.type); | ||
case isContextConsumer(element): | ||
return `${element.type._context.displayName || 'Context'}.Consumer`; | ||
case isContextProvider(element): | ||
return `${element.type._context.displayName || 'Context'}.Provider`; | ||
case isLazy(element): | ||
return 'Lazy'; | ||
case isProfiler(element): | ||
return 'Profiler'; | ||
case isStrictMode(element): | ||
return 'StrictMode'; | ||
case isSuspense(element): | ||
return 'Suspense'; | ||
default: | ||
// this is "unknown" case and will most likely result in `[object Object]` | ||
// it's fine though as we can't display anything meaningful anyway here | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me know if you prefer returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like your idea to return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
return String(element); | ||
} | ||
}; | ||
|
||
const noChildren = (propsValue, propName) => propName !== 'children'; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why you have to change that in the context of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commonjs
plugin is only needed when you actually want to bundle a commonjs dependency - you don't really want that withreact
since you are leaving it as external (like you should), so this was actually a "dead"/redundant configThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's improve this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, could you be more specific? maybe I've explained this poorly - so let me rephrase: the removed code that was not actually used and it wasn't doing anything here. And how this config now is defined is correct - from my point of view this change is "sound" and correct. There is no regression here so this can be merged "as is". Please let me know if this addresses your concerns - if not I can try to explain it further and in more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ho! Sorry I just wanted to said Let's change this! in the meaning of accepting your change with pleasure. Sorry for the wrong wording 🙊