Skip to content

Commit 7ec0431

Browse files
author
vvo
committed
fix(Template): stop leaking data="[object Object]" attributes in production builds
Avoid `<div data="[object Object]">` being inserted in the DOM in production builds. The bug was introduced by fcfb990 where we started using transform-react-remove-prop-types. We cannot rely on PropTypes in production env because they are removed. We ask the dev to be explicit in what he's doing: use rootProps versus forwarding everything a bit magically. fixes #899
1 parent b20e8ba commit 7ec0431

File tree

9 files changed

+35
-54
lines changed

9 files changed

+35
-54
lines changed

‎src/components/Hits.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import map from 'lodash/collection/map';
44
import Template from './Template.js';
55

66
import isEqual from 'lodash/lang/isEqual';
7+
import cx from 'classnames';
78

89
class Hits extends React.Component {
910
shouldComponentUpdate(nextProps) {
@@ -16,9 +17,9 @@ class Hits extends React.Component {
1617
let renderedHits = map(this.props.results.hits, hit => {
1718
return (
1819
<Template
19-
cssClass={this.props.cssClasses.item}
2020
data={hit}
2121
key={hit.objectID}
22+
rootProps={{className: this.props.cssClasses.item}}
2223
templateKey="item"
2324
{...this.props.templateProps}
2425
/>
@@ -31,20 +32,19 @@ class Hits extends React.Component {
3132
renderAllResults() {
3233
return (
3334
<Template
34-
cssClass={this.props.cssClasses.allItems}
3535
data={this.props.results}
36+
rootProps={{className: this.props.cssClasses.allItems}}
3637
templateKey="allItems"
3738
{...this.props.templateProps}
3839
/>
3940
);
4041
}
4142

4243
renderNoResults() {
43-
let className = this.props.cssClasses.root + ' ' + this.props.cssClasses.empty;
4444
return (
4545
<Template
46-
cssClass={className}
4746
data={this.props.results}
47+
rootProps={{className: cx(this.props.cssClasses.root, this.props.cssClasses.empty)}}
4848
templateKey="empty"
4949
{...this.props.templateProps}
5050
/>

‎src/components/RefinementList/RefinementList.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class RefinementList extends React.Component {
1313
isShowMoreOpen: false
1414
};
1515
this.handleItemClick = this.handleItemClick.bind(this);
16+
this.handleClickShowMore = this.handleClickShowMore.bind(this);
1617
}
1718

1819
shouldComponentUpdate(nextProps, nextState) {
@@ -129,7 +130,7 @@ class RefinementList extends React.Component {
129130
const showMoreBtn =
130131
this.props.showMore ?
131132
<Template
132-
onClick={() => this.handleClickShowMore()}
133+
rootProps={{onClick: this.handleClickShowMore}}
133134
templateKey={'show-more-' + (this.state.isShowMoreOpen ? 'active' : 'inactive')}
134135
{...this.props.templateProps}
135136
/> :

‎src/components/Template.js‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import React from 'react';
22

33
import curry from 'lodash/function/curry';
44
import cloneDeep from 'lodash/lang/cloneDeep';
5-
import keys from 'lodash/object/keys';
6-
import omit from 'lodash/object/omit';
75
import mapValues from 'lodash/object/mapValues';
86

97
import hogan from 'hogan.js';
@@ -37,30 +35,17 @@ class Template extends React.Component {
3735
return null;
3836
}
3937

40-
const otherProps = omit(this.props, keys(Template.propTypes));
41-
4238
if (React.isValidElement(content)) {
43-
return (
44-
<div
45-
{...otherProps}
46-
className={this.props.cssClass}
47-
>{content}</div>
48-
);
39+
return <div {...this.props.rootProps}>{content}</div>;
4940
}
5041

51-
return (
52-
<div
53-
{...otherProps}
54-
className={this.props.cssClass}
55-
dangerouslySetInnerHTML={{__html: content}}
56-
/>
57-
);
42+
return <div {...this.props.rootProps} dangerouslySetInnerHTML={{__html: content}} />;
5843
}
5944
}
6045

6146
Template.propTypes = {
62-
cssClass: React.PropTypes.string,
6347
data: React.PropTypes.object,
48+
rootProps: React.PropTypes.object,
6449
templateKey: React.PropTypes.string,
6550
templates: React.PropTypes.objectOf(React.PropTypes.oneOfType([
6651
React.PropTypes.string,

‎src/components/__tests__/Hits-test.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ describe('Hits', () => {
4343
expect(out).toEqualJSX(
4444
<div className="custom-root">
4545
<Template
46-
cssClass="custom-item"
4746
data={results.hits[0]}
4847
key={results.hits[0].objectID}
48+
rootProps={{className: 'custom-item'}}
4949
templateKey="item"
5050
/>
5151
<Template
52-
cssClass="custom-item"
5352
data={results.hits[1]}
5453
key={results.hits[1].objectID}
54+
rootProps={{className: 'custom-item'}}
5555
templateKey="item"
5656
/>
5757
</div>
@@ -86,8 +86,8 @@ describe('Hits', () => {
8686

8787
expect(out).toEqualJSX(
8888
<Template
89-
cssClass="custom-item"
9089
data={results}
90+
rootProps={{className: 'custom-item'}}
9191
templateKey="allItems"
9292
{...templateProps2}
9393
/>
@@ -111,8 +111,8 @@ describe('Hits', () => {
111111

112112
expect(out).toEqualJSX(
113113
<Template
114-
cssClass="custom-root custom-empty"
115114
data={results}
115+
rootProps={{className: 'custom-root custom-empty'}}
116116
templateKey="empty"
117117
/>
118118
);

‎src/components/__tests__/Template-test.js‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('Template', () => {
3232
const out = renderer.getRenderOutput();
3333

3434
const content = 'it works with strings';
35-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: content}}></div>);
35+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: content}}></div>);
3636
});
3737

3838
it('supports templates as functions returning a string', () => {
@@ -45,7 +45,7 @@ describe('Template', () => {
4545
const out = renderer.getRenderOutput();
4646

4747
const content = 'it also works with functions';
48-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: content}}></div>);
48+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: content}}></div>);
4949
});
5050

5151
it('supports templates as functions returning a React element', () => {
@@ -58,7 +58,7 @@ describe('Template', () => {
5858
const out = renderer.getRenderOutput();
5959

6060
const content = 'it also works with functions';
61-
expect(out).toEqualJSX(<div className={undefined}><p>{content}</p></div>);
61+
expect(out).toEqualJSX(<div><p>{content}</p></div>);
6262
});
6363

6464
it('can configure compilation options', () => {
@@ -73,14 +73,11 @@ describe('Template', () => {
7373
const out = renderer.getRenderOutput();
7474

7575
const content = 'it configures compilation delimiters';
76-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: content}}></div>);
76+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: content}}></div>);
7777
});
7878
});
7979

8080
describe('using helpers', () => {
81-
beforeEach(() => {
82-
});
83-
8481
it('call the relevant function', () => {
8582
const props = getProps({
8683
templates: {test: 'it supports {{#helpers.emphasis}}{{feature}}{{/helpers.emphasis}}'},
@@ -92,7 +89,7 @@ describe('Template', () => {
9289
const out = renderer.getRenderOutput();
9390

9491
const content = 'it supports <em>helpers</em>';
95-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: content}}></div>);
92+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: content}}></div>);
9693
});
9794

9895
it('sets the function context (`this`) to the template `data`', done => {
@@ -130,7 +127,7 @@ describe('Template', () => {
130127

131128
const out = renderer.getRenderOutput();
132129
const content = 'it supports transformData';
133-
const expectedJSX = <div className={undefined} dangerouslySetInnerHTML={{__html: content}}></div>;
130+
const expectedJSX = <div dangerouslySetInnerHTML={{__html: content}}></div>;
134131

135132
expect(out).toEqualJSX(expectedJSX);
136133
});
@@ -216,21 +213,19 @@ describe('Template', () => {
216213
});
217214
});
218215

219-
describe('misc feature', () => {
220-
it('accepts props that are not defined in the proptypes', () => {
221-
function fn() {}
216+
it('forward rootProps to the first node', () => {
217+
function fn() {}
222218

223-
const props = getProps({});
224-
renderer.render(<Template onClick={fn} {...props}/>);
219+
const props = getProps({});
220+
renderer.render(<Template rootProps={{className: 'hey', onClick: fn}} {...props}/>);
225221

226-
const out = renderer.getRenderOutput();
227-
const expectedProps = {
228-
className: undefined,
229-
dangerouslySetInnerHTML: {__html: ''},
230-
onClick: fn
231-
};
232-
expect(out).toEqualJSX(<div {...expectedProps}></div>);
233-
});
222+
const out = renderer.getRenderOutput();
223+
const expectedProps = {
224+
className: 'hey',
225+
dangerouslySetInnerHTML: {__html: ''},
226+
onClick: fn
227+
};
228+
expect(out).toEqualJSX(<div {...expectedProps}></div>);
234229
});
235230

236231
context('shouldComponentUpdate', () => {

‎src/decorators/__tests__/headerFooter-test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('headerFooter', () => {
5757
};
5858
expect(out).toEqualJSX(
5959
<div className="ais-root root">
60-
<Template cssClass="ais-header" {...templateProps} onClick={null} />
60+
<Template rootProps={{className: 'ais-header'}} {...templateProps} onClick={null} />
6161
<div className="ais-body body">
6262
<TestComponent {...defaultProps} />
6363
</div>
@@ -86,7 +86,7 @@ describe('headerFooter', () => {
8686
<div className="ais-body body">
8787
<TestComponent {...defaultProps} />
8888
</div>
89-
<Template cssClass="ais-footer" {...templateProps} onClick={null} />
89+
<Template rootProps={{className: 'ais-footer'}} {...templateProps} onClick={null} />
9090
</div>
9191
);
9292
});

‎src/decorators/headerFooter.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ function headerFooter(ComposedComponent) {
4040
let className = cx(this.props.cssClasses[type], `ais-${type}`);
4141
return (
4242
<Template {...this.props.templateProps}
43-
cssClass={className}
4443
onClick={handleClick}
44+
rootProps={{className}}
4545
templateKey={type}
4646
transformData={null}
4747
/>

‎src/widgets/refinement-list/__tests__/refinement-list-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ describe('refinementList()', () => {
197197
};
198198
renderer.render(<Template data={{count: 1000}} {...props} templateKey="item" />);
199199
let out = renderer.getRenderOutput();
200-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: '<label class="">\n <input type="checkbox" class="" value="" />\n <span class="">1,000</span>\n</label>'}} />);
200+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: '<label class="">\n <input type="checkbox" class="" value="" />\n <span class="">1,000</span>\n</label>'}} />);
201201
});
202202

203203
context('cssClasses', () => {

‎src/widgets/toggle/__tests__/toggle-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ describe('toggle()', () => {
141141
templateProps.templatesConfig = {helpers};
142142
renderer.render(<Template data={{count: 1000}} {...templateProps} templateKey="item" />);
143143
let out = renderer.getRenderOutput();
144-
expect(out).toEqualJSX(<div className={undefined} dangerouslySetInnerHTML={{__html: '<label class="">\n <input type="checkbox" class="" value="" />\n <span class="">1,000</span>\n</label>'}} />);
144+
expect(out).toEqualJSX(<div dangerouslySetInnerHTML={{__html: '<label class="">\n <input type="checkbox" class="" value="" />\n <span class="">1,000</span>\n</label>'}} />);
145145
});
146146

147147
it('understands cssClasses', () => {

0 commit comments

Comments
 (0)