-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add .debug() method to ReactWrapper #158
Conversation
|
||
export function debugInst(inst, indentLength = 2) { | ||
if (typeof inst === 'string' || typeof inst === 'number') return escape(inst); | ||
if (!inst) return ''; |
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.
just want to confirm - these two lines will not do anything special for true
, or for Symbols.
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.
hmm.... not that I'm aware of.
I don't actually know what calling this on <div>{true}</div>
would do, but it should do whatever react itself does...
No idea how or why or what symbols would be doing getting called in this function....?
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.
just checked and <div>{true}</div>
gets rendered as <div />
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.
In that case, wouldn't we want if (inst === true) return ''
? or am i misunderstanding something.
what about <div>{Symbol('foo')}</div>
?
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.
@ljharb <div>{Symbol('foo')}</div>
also renderes an empty div. I think this function isn't being used exactly how you imagine it. Though i'm not sure I event 100% understand it... lol
05c16c1
to
067f766
Compare
if (isDOMComponent(publicInst)) { | ||
const renderedChildren = renderedChildrenOfInst(inst) || childrenOfNode(currentElement); | ||
let key; | ||
for (key in renderedChildren) { |
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.
Instead of
let key;
for (key in renderedChildren) {
why not
for (const key in renderedChildren) {
?
Or, can we use Object.values
yet?
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.
Or would Array.from
work here? I'm not really familiar with what _renderedChildren
looks like.
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.
Object.values might work. This ends up not being an array. Not really sure why though. A lot of this code started out from source copied from react core.
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.
You can pull in https://npmjs.com/object.values if you want to use it ;-)
067f766
to
1c8ab61
Compare
👍 we can leverage it in chai-enzyme cc @ayrton |
89371df
to
ea4afc9
Compare
Add .debug() method to ReactWrapper
to: @lencioni @ljharb @goatslacker
Adds a debug method to the ReactWrapper. Mirrors the API and implementation of the method on ShallowWrapper.
Fixes #128