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

Adds HtmlElement pretty-format plugin. #3230

Merged
merged 13 commits into from
Apr 17, 2017
99 changes: 99 additions & 0 deletions packages/pretty-format/src/__tests__/HTMLElementPlugin-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @jest-environment jsdom
*/
/* eslint-disable max-len */
/* eslint-env browser*/

'use strict';

const HTMLElementPlugin = require('../plugins/HTMLElement');
const toPrettyPrintTo = require('./expect-util').getPrettyPrint([
HTMLElementPlugin,
]);

expect.extend({toPrettyPrintTo});

describe('HTMLElement Plugin', () => {
it('supports a single HTML element', () => {
expect(document.createElement('div')).toPrettyPrintTo('<HTMLDivElement />');
});

it('supports an HTML element with a class property', () => {
const parent = document.createElement('div');
parent.className = 'classy';

expect(parent).toPrettyPrintTo('<HTMLDivElement\n class="classy"\n/>');
});

it('supports an HTML element with a title property', () => {
const parent = document.createElement('div');
parent.title = 'title text';

expect(parent).toPrettyPrintTo('<HTMLDivElement\n title="title text"\n/>');
});

it('supports an HTML element with a single attribute', () => {
const parent = document.createElement('div');
parent.setAttribute('class', 'classy');

expect(parent).toPrettyPrintTo('<HTMLDivElement\n class="classy"\n/>');
});

it('supports an HTML element with multiple attributes', () => {
const parent = document.createElement('div');
parent.setAttribute('id', 123);
parent.setAttribute('class', 'classy');

expect(
parent,
).toPrettyPrintTo('<HTMLDivElement\n id="123"\n class="classy"\n/>', {});
});

it('supports an element with text content', () => {
const parent = document.createElement('div');
parent.innerHTML = 'texty texty';

expect(parent).toPrettyPrintTo(
'<HTMLDivElement>\n texty texty\n</HTMLDivElement>',
);
});

it('supports nested elements', () => {
const parent = document.createElement('div');
const child = document.createElement('span');
parent.appendChild(child);
expect(parent).toPrettyPrintTo(
'<HTMLDivElement>\n <HTMLSpanElement />\n</HTMLDivElement>',
);
});

it('supports nested elements with attributes', () => {
const parent = document.createElement('div');
const child = document.createElement('span');
parent.appendChild(child);

child.setAttribute('id', 123);
child.setAttribute('class', 'classy');

expect(parent).toPrettyPrintTo(
'<HTMLDivElement>\n <HTMLSpanElement\n id="123"\n class="classy"\n />\n</HTMLDivElement>',
);
});

it('supports nested elements with text content', () => {
const parent = document.createElement('div');
const child = document.createElement('span');
parent.appendChild(child);
child.textContent = 'texty texty';

expect(parent).toPrettyPrintTo(
'<HTMLDivElement>\n <HTMLSpanElement>\n texty texty\n </HTMLSpanElement>\n</HTMLDivElement>',
);
});
});
47 changes: 6 additions & 41 deletions packages/pretty-format/src/__tests__/ImmutablePlugins-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,52 +10,17 @@
'use strict';

const React = require('react');
const diff = require('jest-diff');
const prettyFormat = require('../');
const Immutable = require('immutable');
const ReactElementPlugin = require('../plugins/ReactElement');
const ReactTestComponentPlugin = require('../plugins/ReactTestComponent');
const ImmutablePlugins = require('../plugins/ImmutablePlugins');
const toPrettyPrintTo = require('./expect-util').getPrettyPrint([
ReactElementPlugin,
ReactTestComponentPlugin,
...ImmutablePlugins,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Node 4 fails because of this destructuring. You can use concat instead

]);

expect.extend({
toPrettyPrintTo(received, expected, opts) {
const prettyPrintImmutable = prettyFormat(
received,
Object.assign(
{
plugins: [ReactElementPlugin, ReactTestComponentPlugin].concat(
ImmutablePlugins,
),
},
opts,
),
);
const pass = prettyPrintImmutable === expected;

const message = pass
? () =>
this.utils.matcherHint('.not.toBe') +
'\n\n' +
`Expected value to not be:\n` +
` ${this.utils.printExpected(expected)}\n` +
`Received:\n` +
` ${this.utils.printReceived(prettyPrintImmutable)}`
: () => {
const diffString = diff(expected, prettyPrintImmutable, {
expand: this.expand,
});
return this.utils.matcherHint('.toBe') +
'\n\n' +
`Expected value to be:\n` +
` ${this.utils.printExpected(expected)}\n` +
`Received:\n` +
` ${this.utils.printReceived(prettyPrintImmutable)}` +
(diffString ? `\n\nDifference:\n\n${diffString}` : '');
};

return {actual: prettyPrintImmutable, message, pass};
},
});
expect.extend({toPrettyPrintTo});

describe('Immutable.OrderedSet plugin', () => {
it('supports an empty set', () => {
Expand Down
52 changes: 52 additions & 0 deletions packages/pretty-format/src/__tests__/expect-util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*/

'use strict';

const diff = require('jest-diff');
const prettyFormat = require('../');

module.exports = {
getPrettyPrint: plugins =>
(received, expected, opts) => {
const prettyFormatted = prettyFormat(
received,
Object.assign(
{
plugins,
},
opts,
),
);
const pass = prettyFormatted === expected;

const message = pass
? () =>
this.utils.matcherHint('.not.toBe') +
'\n\n' +
`Expected value to not be:\n` +
` ${this.utils.printExpected(expected)}\n` +
`Received:\n` +
` ${this.utils.printReceived(prettyFormatted)}`
: () => {
const diffString = diff(expected, prettyFormatted, {
expand: this.expand,
});
return this.utils.matcherHint('.toBe') +
'\n\n' +
`Expected value to be:\n` +
` ${this.utils.printExpected(expected)}\n` +
`Received:\n` +
` ${this.utils.printReceived(prettyFormatted)}` +
(diffString ? `\n\nDifference:\n\n${diffString}` : '');
};

return {actual: prettyFormatted, message, pass};
},
};
104 changes: 104 additions & 0 deletions packages/pretty-format/src/plugins/HTMLElement.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @flow
*/

'use strict';

import type {Colors, Indent, Options, Print, Plugin} from '../types.js';

const escapeHTML = require('./lib/escapeHTML');
const HTML_ELEMENT_REGEXP = /(HTML\w*?Element)/;
const test = isHTMLElement;

function isHTMLElement(value: any) {
const toStringed = value.toString();
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit worried about performance here. This will call toString on any value passed to pretty-format. toString may be expensive if it is overwritten on a class instance. Can we do Object.prototype.toString.call?

Copy link
Contributor Author

@mute mute Mar 31, 2017

Choose a reason for hiding this comment

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

That's a really good call -- I'll check how that behaves on an actual JSDOM HTMLElement to verify.

Looking at it again, I think it's possible that we could just remove this toString completely and just rely on the constructor validation, but I was deferring to the existing implementation from the previous PR.

Does that seem reasonable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup, I think we could make it even more performant. According to ElementImpl we could check for property nodeType. Although this prop alone could possibly result in some false positives (nodeType feels not so unique...), it's good for an early check. Only when we know there's such prop, we could perform constructor and regex check:

const test = (value: any) =>
  value !== undefined &&
  value.nodeType === 1 && 
  value.constructor !== undefined &&
  HTML_ELEMENT_REGEXP.test(value.constructor.name);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome -- I've updated the PR with those changes.

I'm not sure what to do about the tests that are failing; do I need to have mercurial configured locally for those?

Copy link
Collaborator

Choose a reason for hiding this comment

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

return HTML_ELEMENT_REGEXP.test(toStringed) ||
(toStringed === '[object Object]' &&
value.constructor !== undefined &&
value.constructor.name !== undefined &&
HTML_ELEMENT_REGEXP.test(value.constructor.name));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is potentially a hot place, please keep perf in mind: https://twitter.com/bmeurer/status/846951275480711168


function printChildren(flatChildren, print, indent, colors, opts) {
return flatChildren
.map(node => {
if (typeof node === 'object') {
return print(node, print, indent, colors, opts);
} else if (typeof node === 'string') {
return colors.content.open + escapeHTML(node) + colors.content.close;
} else {
return print(node);
}
})
.join(opts.edgeSpacing);
}

function printAttributes(attributes, print, indent, colors, opts) {
return attributes
.sort()
.map(attribute => {
return opts.spacing +
indent(colors.prop.open + attribute.name + colors.prop.close + '=') +
colors.value.open +
`"${attribute.value}"` +
colors.value.close;
})
.join('');
}

const print = (
element: any,
print: Print,
indent: Indent,
opts: Options,
colors: Colors,
) => {
let result = colors.tag.open + '<';
const elementName = element.constructor
Copy link
Collaborator

Choose a reason for hiding this comment

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

const elementName = element.tagName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tagName defaults to all uppercase, which felt odd to me -- should I leave it that way or is the .toLowerCase() call okay here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

lower case is better, leave it as is :)

? element.constructor.name
: 'HTMLElement';
result += elementName + colors.tag.close;

const hasAttributes = element.attributes && element.attributes.length;
if (hasAttributes) {
const attributes = Array.prototype.slice.call(element.attributes);
result += printAttributes(attributes, print, indent, colors, opts);
}

const flatChildren = Array.prototype.slice.call(element.children);
if (!flatChildren.length && element.textContent) {
flatChildren.push(element.textContent);
}

const closeInNewLine = hasAttributes && !opts.min;
if (flatChildren.length) {
const children = printChildren(flatChildren, print, indent, colors, opts);
result += colors.tag.open +
(closeInNewLine ? '\n' : '') +
'>' +
colors.tag.close +
opts.edgeSpacing +
indent(children) +
opts.edgeSpacing +
colors.tag.open +
'</' +
elementName +
'>' +
colors.tag.close;
} else {
result += colors.tag.open +
(closeInNewLine ? '\n' : ' ') +
'/>' +
colors.tag.close;
}

return result;
};

module.exports = ({print, test}: Plugin);