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

Add alternative serialize API for pretty-format plugins #4114

Merged
merged 12 commits into from
Aug 1, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
exports[`ReactElement plugin highlights syntax 1`] = `
"<Mouse</>
prop</>=<green>{
<div</>></>
<div></>
</>mouse</>
<span</>></>
<span></>
</>rat</>
</span></>
</div></>
Expand All @@ -24,9 +24,9 @@ exports[`ReactElement plugin highlights syntax with color from theme option 1`]
exports[`ReactTestComponent plugin highlights syntax 1`] = `
"<Mouse</>
prop</>=<green>{
<div</>></>
<div></>
</>mouse</>
<span</>></>
<span></>
</>rat</>
</span></>
</div></>
Expand Down
85 changes: 80 additions & 5 deletions packages/pretty-format/src/__tests__/pretty_format.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,45 @@ describe('prettyFormat()', () => {
expect(prettyFormat('\\ \\\\')).toEqual('"\\\\ \\\\\\\\"');
});

it('prints a multiline string', () => {
const val = ['line 1', 'line 2', 'line 3'].join('\n');
expect(prettyFormat(val)).toEqual('"' + val + '"');
});

it('prints a multiline string as value of object property', () => {
const polyline = {
props: {
id: 'J',
points: ['0.5,0.460', '0.5,0.875', '0.25,0.875'].join('\n'),
},
type: 'polyline',
};
const val = {
props: {
children: polyline,
},
type: 'svg',
};
expect(prettyFormat(val)).toEqual(
[
'Object {',
' "props": Object {',
' "children": Object {',
' "props": Object {',
' "id": "J",',
' "points": "0.5,0.460',
'0.5,0.875',
'0.25,0.875",',
' },',
' "type": "polyline",',
' },',
' },',
' "type": "svg",',
'}',
].join('\n'),
);
});

it('prints a symbol', () => {
const val = Symbol('symbol');
expect(prettyFormat(val)).toEqual('Symbol(symbol)');
Expand Down Expand Up @@ -321,11 +360,47 @@ describe('prettyFormat()', () => {
);
});

it('can customize indent', () => {
const val = {prop: 'value'};
expect(prettyFormat(val, {indent: 4})).toEqual(
'Object {\n "prop": "value",\n}',
);
describe('indent option', () => {
const val = [
{
id: '8658c1d0-9eda-4a90-95e1-8001e8eb6036',
text: 'Add alternative serialize API for pretty-format plugins',
type: 'ADD_TODO',
},
{
id: '8658c1d0-9eda-4a90-95e1-8001e8eb6036',
type: 'TOGGLE_TODO',
},
];
const expected = [
'Array [',
' Object {',
' "id": "8658c1d0-9eda-4a90-95e1-8001e8eb6036",',
' "text": "Add alternative serialize API for pretty-format plugins",',
' "type": "ADD_TODO",',
' },',
' Object {',
' "id": "8658c1d0-9eda-4a90-95e1-8001e8eb6036",',
' "type": "TOGGLE_TODO",',
' },',
']',
].join('\n');
test('default implicit: 2 spaces', () => {
expect(prettyFormat(val)).toEqual(expected);
});
test('default explicit: 2 spaces', () => {
expect(prettyFormat(val, {indent: 2})).toEqual(expected);
});
test('non-default: 0 spaces', () => {
expect(prettyFormat(val, {indent: 0})).toEqual(
expected.replace(/ {2}/g, ''),
);
});
test('non-default: 4 spaces', () => {
expect(prettyFormat(val, {indent: 4})).toEqual(
expected.replace(/ {2}/g, ' '),
);
});
});

it('can customize the max depth', () => {
Expand Down
154 changes: 131 additions & 23 deletions packages/pretty-format/src/__tests__/react.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,33 @@ const prettyFormat = require('../');
const ReactTestComponent = require('../plugins/react_test_component');
const ReactElement = require('../plugins/react_element');

function assertPrintedJSX(actual, expected, opts) {
expect(
prettyFormat(
actual,
Object.assign(
{
plugins: [ReactElement],
},
opts,
),
const prettyFormatElementPlugin = (element, options) =>
prettyFormat(
element,
Object.assign(
{
plugins: [ReactElement],
},
options,
),
).toEqual(expected);
expect(
prettyFormat(
renderer.create(actual).toJSON(),
Object.assign(
{
plugins: [ReactTestComponent, ReactElement],
},
opts,
),
);

const prettyFormatBothPlugins = (object, options) =>
prettyFormat(
object,
Object.assign(
{
plugins: [ReactTestComponent, ReactElement],
},
options,
),
).toEqual(expected);
);

function assertPrintedJSX(val, formatted, options) {
expect(prettyFormatElementPlugin(val, options)).toEqual(formatted);
expect(
prettyFormatBothPlugins(renderer.create(val).toJSON(), options),
).toEqual(formatted);
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about:

const formattedElementPlugin = prettyFormatElementPlugin(val, options);
const formattedBothPlugins = prettyFormatBothPlugins(renderer.create(val).toJSON(), options),
expect(formattedElementPlugin).toEqual(formattedBothPlugins);

This way we only have one expect and still test the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One expect requires both plugins to return consistent results, which is necessary but not sufficient. I’ll replace formatted with expected to make it clearer why 2 expect are needed:

  • Both plugins could return consistent but incorrect results.
  • If one plugin is incorrect (for example, ReactElement for one falsey child) then a combined expect has a 50% chance of implying the wrong result is expected.

Something less than ideal about assertPrintedJSX is if a test fails, the message can’t identify which plugin :(

The test of array of elements caused me to factor out helper functions which I renamed and used in several test cases near the end:

  • formatElement
  • formatTestObject

}

test('supports a single element with no props or children', () => {
Expand All @@ -49,13 +53,20 @@ test('supports a single element with no props', () => {
);
});

test('supports a single element with number children', () => {
test('supports a single element with nonzero number child', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: non-zero

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you. And I moved the new test for empty string to make the pairs match up.

assertPrintedJSX(
React.createElement('Mouse', null, 4),
'<Mouse>\n 4\n</Mouse>',
);
});

test('supports a single element with zero number child', () => {
assertPrintedJSX(
React.createElement('Mouse', null, 0),
'<Mouse>\n 0\n</Mouse>',
);
});

test('supports a single element with mixed children', () => {
assertPrintedJSX(
React.createElement('Mouse', null, [[1, null], 2, undefined, [false, [3]]]),
Expand All @@ -70,6 +81,28 @@ test('supports props with strings', () => {
);
});

test('supports props with multiline strings', () => {
const val = React.createElement(
'svg',
null,
React.createElement('polyline', {
id: 'J',
points: ['0.5,0.460', '0.5,0.875', '0.25,0.875'].join('\n'),
}),
);
const formatted = [
'<svg>',
' <polyline',
' id="J"',
' points="0.5,0.460',
'0.5,0.875',
'0.25,0.875"',
' />',
'</svg>',
].join('\n');
assertPrintedJSX(val, formatted);
});

test('supports props with numbers', () => {
assertPrintedJSX(
React.createElement('Mouse', {size: 5}),
Expand Down Expand Up @@ -292,7 +325,82 @@ test('supports a single element with React elements with array children', () =>
);
});

test('uses the supplied line seperator for min mode', () => {
test('supports array of elements', () => {
const val = [
React.createElement('dt', null, 'jest'),
React.createElement('dd', null, 'to talk in a playful manner'),
React.createElement(
'dd',
{style: {color: '#99424F'}},
'painless JavaScript testing',
),
];
const formatted = [
'Array [',
' <dt>',
' jest',
' </dt>,',
' <dd>',
' to talk in a playful manner',
' </dd>,',
' <dd',
' style={',
' Object {',
' "color": "#99424F",',
' }',
' }',
' >',
' painless JavaScript testing',
' </dd>,',
']',
].join('\n');
expect(prettyFormatElementPlugin(val)).toEqual(formatted);
expect(
prettyFormatBothPlugins(
val.map(element => renderer.create(element).toJSON()),
),
).toEqual(formatted);
});

describe('indent option', () => {
const val = React.createElement(
'ul',
null,
React.createElement(
'li',
{style: {color: 'green', textDecoration: 'none'}},
'Test indent option',
),
);
const formatted = [
'<ul>',
' <li',
' style={',
' Object {',
' "color": "green",',
' "textDecoration": "none",',
' }',
' }',
' >',
' Test indent option',
' </li>',
'</ul>',
].join('\n');
test('default implicit: 2 spaces', () => {
assertPrintedJSX(val, formatted);
});
test('default explicit: 2 spaces', () => {
assertPrintedJSX(val, formatted, {indent: 2});
});
test('non-default: 0 spaces', () => {
assertPrintedJSX(val, formatted.replace(/ {2}/g, ''), {indent: 0});
});
test('non-default: 4 spaces', () => {
assertPrintedJSX(val, formatted.replace(/ {2}/g, ' '), {indent: 4});
});
});

test('min option', () => {
assertPrintedJSX(
React.createElement(
'Mouse',
Expand Down
51 changes: 27 additions & 24 deletions packages/pretty-format/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,30 +377,33 @@ function printPlugin(
depth: number,
refs: Refs,
): string {
function boundPrint(val) {
return print(val, config, indentation, depth, refs);
}

function boundIndent(str) {
const indentationNext = indentation + config.indent;
return (
indentationNext + str.replace(NEWLINE_REGEXP, '\n' + indentationNext)
);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason why these functions are now inlined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you for asking. To avoid the problem in #3962 where printPlugin made unused closures for the callback functions, especially now that it might call either serialize or print property.

const opts = {
edgeSpacing: config.spacingOuter,
min: config.min,
spacing: config.spacingInner,
};

const printed = plugin.print(
val,
boundPrint,
boundIndent,
opts,
config.colors,
);
const printed = plugin.serialize
? plugin.serialize(
val,
config,
(valChild, indentationChild, depthChild, refsChild) =>
print(valChild, config, indentationChild, depthChild, refsChild),
indentation,
depth,
refs,
)
: plugin.print(
val,
valChild => print(valChild, config, indentation, depth, refs),
str => {
const indentationNext = indentation + config.indent;
return (
indentationNext +
str.replace(NEWLINE_REGEXP, '\n' + indentationNext)
);
},
{
edgeSpacing: config.spacingOuter,
min: config.min,
spacing: config.spacingInner,
},
config.colors,
);
if (typeof printed !== 'string') {
throw new Error(
`pretty-format: Plugin must return type "string" but instead returned "${typeof printed}".`,
Expand Down
Loading