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

Fix #925 - indentation is broken for debug() on shallow wrapper #926

Merged
merged 1 commit into from
May 17, 2017

Conversation

finnigantime
Copy link
Contributor

@finnigantime finnigantime commented May 3, 2017

SUMMARY

DETAILS

The problem is with the use of .map():

export function debugNodes(nodes) {
  return nodes.map(debugNode).join('\n\n\n');
 }

Array.map passes 3 args to the callback, the second of which is the current index (0 in this case since there's only one top-level node), which overrides the default arg value indentLength = 2 in debugNode.

The fix here adds a callback that explicitly only takes one parameter and then calls debugNode explicitly with that single parameter: nodes.map(node => debugNode(node))

ljharb
ljharb previously approved these changes May 3, 2017
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems legit.

@finnigantime
Copy link
Contributor Author

@ljharb Thanks for looking. I pushed an additional commit to skip these new tests on v0.13 where they were failing.

});
});

describeIf(!REACT013, 'debugNodes', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of skipping these tests on react 13, can we just not use an SFC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Good idea, I can try that.

@finnigantime
Copy link
Contributor Author

@ljharb updated

ljharb
ljharb previously approved these changes May 3, 2017
nfcampos
nfcampos previously approved these changes May 6, 2017
@ljharb
Copy link
Member

ljharb commented May 6, 2017

@finnigantime please don't use the update branch button in Github's UI - this needs a rebase on the command line.

@finnigantime finnigantime force-pushed the finnigantime/shallowPrintTree branch from 2f57e62 to dc34bc3 Compare May 16, 2017 16:58
@finnigantime finnigantime force-pushed the finnigantime/shallowPrintTree branch 2 times, most recently from b24424f to 21a5ebf Compare May 16, 2017 17:05
@finnigantime
Copy link
Contributor Author

Should be squashed and rebased against the latest master commit now.

@ljharb
Copy link
Member

ljharb commented May 16, 2017

The tests are failing due to npm v4.6 breaking on node < 1.0. I'll fix in master, after which you can rebase.

@ljharb
Copy link
Member

ljharb commented May 16, 2017

@finnigantime k, if you rebase once more it should pass

@finnigantime finnigantime force-pushed the finnigantime/shallowPrintTree branch from 21a5ebf to f72cae6 Compare May 16, 2017 21:50
@finnigantime
Copy link
Contributor Author

@ljharb Thanks! Green build. G2G?

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

Successfully merging this pull request may close these issues.

indentation is broken for debug() on shallow wrapper
3 participants