Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Warnings on setting propTypes, defaultProps and contextTypes of MobxStoreInjector #88

Closed
wants to merge 2 commits into from
Closed

Warnings on setting propTypes, defaultProps and contextTypes of MobxStoreInjector #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd83fcc
Add warnings on setting properties on Store Injector
jtraub Jul 16, 2016
1507b1a
Add tests for warnings in Injector
jtraub Jul 19, 2016
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
19 changes: 19 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,25 @@
});
Injector.contextTypes = { mobxStores: PropTypes.object };
Injector.wrappedComponent = component;

if (process.env.NODE_ENV !== 'production') {
Copy link
Member

Choose a reason for hiding this comment

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

Can throw error if process is not defined

var warningOnProperties = ['propTypes', 'defaultProps', 'contextTypes'];

warningOnProperties.forEach(function(prop) {
var propValue = Injector[prop];
Object.defineProperty(Injector, prop, {
set: function (_) {
var name = component.displayName || component.name;
console.warn('Mobx Injector: you are trying to attach ' + prop +
' to HOC instead of '+ name +'. Use `wrappedComponent` property.');
},
get: function() {
return propValue;
}
});
});
}

return Injector;
}

Expand Down
83 changes: 80 additions & 3 deletions test/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ test('observer based context', t => {
e("span", {},
e(B, {})
),
e("section", {},
e("section", {},
e(Provider, { foo: 42}, e(B, {}))
)
)
Expand Down Expand Up @@ -132,6 +132,83 @@ test('observer based context', t => {
t.end();
})


test('warning is printed when attaching propTypes/defaultProps/contextTypes to HOC not in production', t => {
var msg = [];
var baseWarn = console.warn;
console.warn = (m) => msg.push(m);

var C = observer(["foo"], React.createClass({
displayName: 'C',
render: function() {
return e("div", {}, "context:" + this.props.foo);
}
}));

C.propTypes = {};
C.defaultProps = {};
C.contextTypes = {};

var B = React.createClass({
render: function() {
return e(C, {});
}
});

var A = React.createClass({
render: function() {
return e(Provider, { foo: "bar" }, e(B, {}))
}
})

const wrapper = mount(e(A));
t.equal(msg.length, 3);
t.equal(msg[0], "Mobx Injector: you are trying to attach propTypes to HOC instead of C. Use `wrappedComponent` property.");
t.equal(msg[1], "Mobx Injector: you are trying to attach defaultProps to HOC instead of C. Use `wrappedComponent` property.");
t.equal(msg[2], "Mobx Injector: you are trying to attach contextTypes to HOC instead of C. Use `wrappedComponent` property.");

console.warn = baseWarn;
t.end();
})


test('warning is not printed when attaching propTypes to HOC in production', t => {
var msg = [];
var baseWarn = console.warn;
console.warn = (m) => msg = m;

var env = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';


var C = observer(["foo"], React.createClass({
displayName: 'C',
render: function() {
return e("div", {}, "context:" + this.props.foo);
}
}));

C.propTypes = {};

var B = React.createClass({
render: function() {
return e(C, {});
}
});

var A = React.createClass({
render: function() {
return e(Provider, { foo: "bar" }, e(B, {}))
}
})

const wrapper = mount(e(A));
t.equal(msg.length, 0);
console.warn = baseWarn;
process.env.NODE_ENV = env;
t.end();
})

test('warning is printed when changing stores', t => {
var msg;
var baseWarn = console.warn;
Expand All @@ -156,7 +233,7 @@ test('observer based context', t => {
return e("section", {},
e("span", {}, a.get()),
e(Provider, { foo: a.get() }, e(B, {}))
);
);
}
}))

Expand All @@ -176,4 +253,4 @@ test('observer based context', t => {
})

t.end()
})
})
83 changes: 80 additions & 3 deletions test/inject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ test('inject based context', t => {
e("span", {},
e(B, {})
),
e("section", {},
e("section", {},
e(Provider, { foo: 42}, e(B, {}))
)
)
Expand Down Expand Up @@ -157,7 +157,7 @@ test('inject based context', t => {
return e("section", {},
e("span", {}, a.get()),
e(Provider, { foo: a.get() }, e(B, {}))
);
);
}
}))

Expand All @@ -176,6 +176,83 @@ test('inject based context', t => {
t.end();
})

test('warning is printed when attaching propTypes/defaultProps/contextTypes to HOC not in production', t => {
var msg = [];
var baseWarn = console.warn;
console.warn = (m) => msg.push(m);

var C = inject(["foo"])(React.createClass({
displayName: 'C',
render: function() {
return e("div", {}, "context:" + this.props.foo);
}
}));

C.propTypes = {};
C.defaultProps = {};
C.contextTypes = {};

var B = React.createClass({
render: function() {
return e(C, {});
}
});

var A = React.createClass({
render: function() {
return e(Provider, { foo: "bar" }, e(B, {}))
}
})

const wrapper = mount(e(A));
t.equal(msg.length, 3);
t.equal(msg[0], "Mobx Injector: you are trying to attach propTypes to HOC instead of C. Use `wrappedComponent` property.");
t.equal(msg[1], "Mobx Injector: you are trying to attach defaultProps to HOC instead of C. Use `wrappedComponent` property.");
t.equal(msg[2], "Mobx Injector: you are trying to attach contextTypes to HOC instead of C. Use `wrappedComponent` property.");

console.warn = baseWarn;
t.end();
})


test('warning is not printed when attaching propTypes to HOC in production', t => {
var msg = [];
var baseWarn = console.warn;
console.warn = (m) => msg = m;

var env = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';


var C = inject(["foo"])(React.createClass({
displayName: 'C',
render: function() {
return e("div", {}, "context:" + this.props.foo);
}
}));

C.propTypes = {};

var B = React.createClass({
render: function() {
return e(C, {});
}
});

var A = React.createClass({
render: function() {
return e(Provider, { foo: "bar" }, e(B, {}))
}
})

const wrapper = mount(e(A));
t.equal(msg.length, 0);
console.warn = baseWarn;
process.env.NODE_ENV = env;
t.end();
})


test('custom storesToProps', t => {
var C = inject(
function(stores, props, context) {
Expand Down Expand Up @@ -224,4 +301,4 @@ test('inject based context', t => {
})

t.end()
})
})