Skip to content

Commit

Permalink
fix warnings to be per-component, rather than per file
Browse files Browse the repository at this point in the history
This ensures that all warnings are emitted properly and it isn't
a whack-a-mole situation where you fix one and then another
pops up you weren't aware of.
  • Loading branch information
quantizor committed Nov 13, 2018
1 parent e5681af commit 1f7a08d
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 85 deletions.
82 changes: 46 additions & 36 deletions src/models/StyledComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,32 +40,6 @@ function generateId(_ComponentStyle: Function, _displayName: string, parentCompo
return parentComponentId ? `${parentComponentId}-${componentId}` : componentId;
}

const warnInnerRef = once(() =>
// eslint-disable-next-line no-console
console.warn(
'The "innerRef" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use "ref" instead like a typical component.'
)
);

const warnAttrsFnObjectKeyDeprecated = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`Functions as object-form attrs({}) keys are now deprecated and will be removed in a future version of styled-components. Switch to the new attrs(props => ({})) syntax instead for easier and more powerful composition. The attrs key in question is "${key}" on component "${displayName}".`
)
);

const warnNonStyledComponentAttrsObjectKey = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`It looks like you've used a non styled-component as the value for the "${key}" prop in an object-form attrs constructor of "${displayName}".\n` +
'You should use the new function-form attrs constructor which avoids this issue: attrs(props => ({ yourStuff }))\n' +
"To continue using the deprecated object syntax, you'll need to wrap your component prop in a function to make it available inside the styled component (you'll still get the deprecation warning though.)\n" +
`For example, { ${key}: () => InnerComponent } instead of { ${key}: InnerComponent }`
)
);

// $FlowFixMe
class StyledComponent extends Component<*> {
renderOuter: Function;
Expand All @@ -74,13 +48,47 @@ class StyledComponent extends Component<*> {

styleSheet: ?StyleSheet;

warnInnerRef: Function;

warnAttrsFnObjectKeyDeprecated: Function;

warnNonStyledComponentAttrsObjectKey: Function;

attrs = {};

constructor() {
super();
this.renderOuter = this.renderOuter.bind(this);
this.renderInner = this.renderInner.bind(this);

if (process.env.NODE_ENV !== 'production') {
this.warnInnerRef = once(displayName =>
// eslint-disable-next-line no-console
console.warn(
`The "innerRef" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use "ref" instead like a typical component. "innerRef" was detected on component "${displayName}".`
)
);

this.warnAttrsFnObjectKeyDeprecated = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`Functions as object-form attrs({}) keys are now deprecated and will be removed in a future version of styled-components. Switch to the new attrs(props => ({})) syntax instead for easier and more powerful composition. The attrs key in question is "${key}" on component "${displayName}".`
)
);

this.warnNonStyledComponentAttrsObjectKey = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`It looks like you've used a non styled-component as the value for the "${key}" prop in an object-form attrs constructor of "${displayName}".\n` +
'You should use the new function-form attrs constructor which avoids this issue: attrs(props => ({ yourStuff }))\n' +
"To continue using the deprecated object syntax, you'll need to wrap your component prop in a function to make it available inside the styled component (you'll still get the deprecation warning though.)\n" +
`For example, { ${key}: () => InnerComponent } instead of { ${key}: InnerComponent }`
)
);
}

if (process.env.NODE_ENV !== 'production' && IS_BROWSER) {
classNameUsageCheckInjector(this);
}
Expand All @@ -101,7 +109,13 @@ class StyledComponent extends Component<*> {
}

renderInner(theme?: Theme) {
const { componentStyle, defaultProps, styledComponentId, target } = this.props.forwardedClass;
const {
componentStyle,
defaultProps,
displayName,
styledComponentId,
target,
} = this.props.forwardedClass;

let generatedClassName;
if (componentStyle.isStatic) {
Expand Down Expand Up @@ -129,7 +143,7 @@ class StyledComponent extends Component<*> {
// eslint-disable-next-line guard-for-in
for (key in computedProps) {
if (process.env.NODE_ENV !== 'production' && key === 'innerRef' && isTargetTag) {
warnInnerRef();
this.warnInnerRef(displayName);
}

if (key === 'forwardedClass' || key === 'as') continue;
Expand Down Expand Up @@ -182,18 +196,14 @@ class StyledComponent extends Component<*> {

if (!attrDefWasFn) {
if (isFunction(attr) && !isDerivedReactComponent(attr) && !isStyledComponent(attr)) {
if (process.env.NODE_ENV !== 'production' && warnAttrsFnObjectKeyDeprecated) {
warnAttrsFnObjectKeyDeprecated(key, props.forwardedClass.displayName);
if (process.env.NODE_ENV !== 'production') {
this.warnAttrsFnObjectKeyDeprecated(key, props.forwardedClass.displayName);
}

attr = attr(context);

if (
process.env.NODE_ENV !== 'production' &&
React.isValidElement(attr) &&
warnNonStyledComponentAttrsObjectKey
) {
warnNonStyledComponentAttrsObjectKey(key, props.forwardedClass.displayName);
if (process.env.NODE_ENV !== 'production' && React.isValidElement(attr)) {
this.warnNonStyledComponentAttrsObjectKey(key, props.forwardedClass.displayName);
}
}
}
Expand Down
83 changes: 47 additions & 36 deletions src/models/StyledNativeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,50 @@ import { ThemeConsumer } from './ThemeProvider';
import type { Theme } from './ThemeProvider';
import type { Attrs, RuleSet, Target } from '../types';

const warnInnerRef = once(() =>
// eslint-disable-next-line no-console
console.warn(
'The "innerRef" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use "ref" instead like a typical component.'
)
);

const warnAttrsFnObjectKeyDeprecated = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`Functions as object-form attrs({}) keys are now deprecated and will be removed in a future version of styled-components. Switch to the new attrs(props => ({})) syntax instead for easier and more powerful composition. The attrs key in question is "${key}" on component "${displayName}".`
)
);

const warnNonStyledComponentAttrsObjectKey = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`It looks like you've used a non styled-component as the value for the "${key}" prop in an object-form attrs constructor of "${displayName}".\n` +
'You should use the new function-form attrs constructor which avoids this issue: attrs(props => ({ yourStuff }))\n' +
"To continue using the deprecated object syntax, you'll need to wrap your component prop in a function to make it available inside the styled component (you'll still get the deprecation warning though.)\n" +
`For example, { ${key}: () => InnerComponent } instead of { ${key}: InnerComponent }`
)
);

// $FlowFixMe
class StyledNativeComponent extends Component<*, *> {
root: ?Object;

warnInnerRef: Function;

warnAttrsFnObjectKeyDeprecated: Function;

warnNonStyledComponentAttrsObjectKey: Function;

attrs = {};

constructor(props) {
super(props);

if (process.env.NODE_ENV !== 'production') {
this.warnInnerRef = once(displayName =>
// eslint-disable-next-line no-console
console.warn(
`The "innerRef" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use "ref" instead like a typical component. "innerRef" was detected on component "${displayName}".`
)
);

this.warnAttrsFnObjectKeyDeprecated = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`Functions as object-form attrs({}) keys are now deprecated and will be removed in a future version of styled-components. Switch to the new attrs(props => ({})) syntax instead for easier and more powerful composition. The attrs key in question is "${key}" on component "${displayName}".`
)
);

this.warnNonStyledComponentAttrsObjectKey = once(
(key, displayName): void =>
// eslint-disable-next-line no-console
console.warn(
`It looks like you've used a non styled-component as the value for the "${key}" prop in an object-form attrs constructor of "${displayName}".\n` +
'You should use the new function-form attrs constructor which avoids this issue: attrs(props => ({ yourStuff }))\n' +
"To continue using the deprecated object syntax, you'll need to wrap your component prop in a function to make it available inside the styled component (you'll still get the deprecation warning though.)\n" +
`For example, { ${key}: () => InnerComponent } instead of { ${key}: InnerComponent }`
)
);
}
}

render() {
return (
<ThemeConsumer>
Expand All @@ -59,7 +71,7 @@ class StyledNativeComponent extends Component<*, *> {
...props
} = this.props;

const { defaultProps, target } = forwardedClass;
const { defaultProps, displayName, target } = forwardedClass;

let generatedStyles;
if (theme !== undefined) {
Expand All @@ -76,7 +88,10 @@ class StyledNativeComponent extends Component<*, *> {
};

if (forwardedRef) propsForElement.ref = forwardedRef;
if (process.env.NODE_ENV !== 'production' && innerRef) warnInnerRef();

if (process.env.NODE_ENV !== 'production' && innerRef) {
this.warnInnerRef(displayName);
}

return createElement(renderAs || target, propsForElement);
}}
Expand Down Expand Up @@ -110,18 +125,14 @@ class StyledNativeComponent extends Component<*, *> {

if (!attrDefWasFn) {
if (isFunction(attr) && !isDerivedReactComponent(attr) && !isStyledComponent(attr)) {
if (process.env.NODE_ENV !== 'production' && warnAttrsFnObjectKeyDeprecated) {
warnAttrsFnObjectKeyDeprecated(key, this.props.forwardedClass.displayName);
if (process.env.NODE_ENV !== 'production') {
this.warnAttrsFnObjectKeyDeprecated(key, this.props.forwardedClass.displayName);
}

attr = attr(context);

if (
process.env.NODE_ENV !== 'production' &&
React.isValidElement(attr) &&
warnNonStyledComponentAttrsObjectKey
) {
warnNonStyledComponentAttrsObjectKey(key, this.props.forwardedClass.displayName);
if (process.env.NODE_ENV !== 'production' && React.isValidElement(attr)) {
this.warnNonStyledComponentAttrsObjectKey(key, this.props.forwardedClass.displayName);
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/native/test/native.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import TestRenderer from 'react-test-renderer';

import styled, { ThemeProvider } from '../index';

// for the purpose of testing warnings we want to make sure they're always fired
jest.mock('../../utils/once', () => cb => cb);

// NOTE: These tests are like the ones for Web but a "light-version" of them
// This is mostly due to the similar logic

Expand Down
3 changes: 0 additions & 3 deletions src/primitives/test/primitives.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import TestRenderer from 'react-test-renderer';

import styled, { ThemeProvider } from '../index';

// for the purpose of testing warnings we want to make sure they're always fired
jest.mock('../../utils/once', () => cb => cb);

// NOTE: These tests are copy pasted from ../native/test/native.test.js

describe('primitives', () => {
Expand Down
3 changes: 0 additions & 3 deletions src/test/attrs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import { resetStyled, expectCSSMatches } from './utils';

let styled;

// for the purpose of testing warnings we want to make sure they're always fired
jest.mock('../utils/once', () => cb => cb);

describe('attrs', () => {
beforeEach(() => {
jest.spyOn(console, 'warn');
Expand Down
5 changes: 1 addition & 4 deletions src/test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import { find } from '../../test-utils';

let styled;

// for the purpose of testing warnings we want to make sure they're always fired
jest.mock('../utils/once', () => cb => cb);

describe('basic', () => {
/**
* Make sure the setup is the same for every test
Expand Down Expand Up @@ -320,7 +317,7 @@ describe('basic', () => {

TestRenderer.create(<Comp innerRef={ref} />);
expect(console.warn.mock.calls[0][0]).toMatchInlineSnapshot(
`"The \\"innerRef\\" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use \\"ref\\" instead like a typical component."`
`"The \\"innerRef\\" API has been removed in styled-components v4 in favor of React 16 ref forwarding, use \\"ref\\" instead like a typical component. \\"innerRef\\" was detected on component \\"styled.div\\"."`
);
});

Expand Down

0 comments on commit 1f7a08d

Please sign in to comment.