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

[styles] Remove hoisting of static properties in HOCs #13698

Merged
merged 6 commits into from
Dec 14, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ module.exports = [
name: 'The main docs bundle',
webpack: false,
path: main.path,
limit: '176 KB',
limit: '176.1 KB',
},
{
name: 'The docs home page',
Expand Down
1 change: 0 additions & 1 deletion packages/material-ui-styles/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
"@material-ui/utils": "^3.0.0-alpha.0",
"classnames": "^2.2.5",
"deepmerge": "^3.0.0",
"hoist-non-react-statics": "^3.2.1",
"jss": "^9.3.3",
"jss-camel-case": "^6.0.0",
"jss-default-unit": "^8.0.2",
Expand Down
18 changes: 18 additions & 0 deletions packages/material-ui-styles/src/hoistInternalStatics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Copies internal immediate statics from material-ui from source to target
*/
export default function hoistStatics(target, source) {
const internals = ['muiName'];

for (let i = 0; i < internals.length; i += 1) {
const key = internals[i];
const descriptor = Object.getOwnPropertyDescriptor(source, key);
try {
Object.defineProperty(target, key, descriptor);
} catch (e) {
// Avoid failures from read-only properties and undefined descriptors
}
}

return target;
}
4 changes: 2 additions & 2 deletions packages/material-ui-styles/src/withStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import PropTypes from 'prop-types';
import warning from 'warning';
import getDynamicStyles from 'jss/lib/utils/getDynamicStyles';
import { getDisplayName } from '@material-ui/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import { increment } from './indexCounter';
import mergeClasses from './mergeClasses';
import multiKeyStore from './multiKeyStore';
import getStylesCreator from './getStylesCreator';
import getThemeProps from './getThemeProps';
import hoistStatics from './hoistInternalStatics';
import { StylesContext } from './StylesProvider';
import { ThemeContext } from './ThemeProvider';

Expand Down Expand Up @@ -333,7 +333,7 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
WithStyles.displayName = `WithStyles(${getDisplayName(Component)})`;
}

hoistNonReactStatics(WithStyles, Component);
hoistStatics(WithStyles, Component);

if (process.env.NODE_ENV !== 'production') {
// Exposed for test purposes.
Expand Down
23 changes: 23 additions & 0 deletions packages/material-ui-styles/src/withStyles.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { assert } from 'chai';
import React from 'react';
import { Input } from '@material-ui/core';
import { isMuiElement } from '@material-ui/core/utils/reactHelpers';
import withStyles from './withStyles';

describe('withStyles', () => {
it('does not hoist statics', () => {
const Test = () => null;
Test.someStatic = 'will not get hoisted';
const TestWithStyles = withStyles({})(Test);
assert.strictEqual(TestWithStyles.someStatic, undefined);
});

it('hoists mui internals', () => {
assert.strictEqual(isMuiElement(<Input />, ['Input']), true);

// the imported Input is decorated with @material-ui/core/styles
const StyledInput = withStyles({})(Input);

assert.strictEqual(isMuiElement(<StyledInput />, ['Input']), true);
});
});
4 changes: 2 additions & 2 deletions packages/material-ui-styles/src/withTheme.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import hoistNonReactStatics from 'hoist-non-react-statics';
import { getDisplayName } from '@material-ui/utils';
import hoistStatics from './hoistInternalStatics';
import { ThemeContext } from './ThemeProvider';

// Provide the theme object as a property to the input component.
Expand All @@ -26,7 +26,7 @@ const withTheme = () => Component => {
WithTheme.displayName = `WithTheme(${getDisplayName(Component)})`;
}

hoistNonReactStatics(WithTheme, Component);
hoistStatics(WithTheme, Component);

return WithTheme;
};
Expand Down
17 changes: 17 additions & 0 deletions packages/material-ui-styles/src/withTheme.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import { assert } from 'chai';
import { createMount } from '@material-ui/core/test-utils';
import { Input } from '@material-ui/core';
import { isMuiElement } from '@material-ui/core/utils/reactHelpers';
import PropTypes from 'prop-types';
import withTheme from './withTheme';
import ThemeProvider from './ThemeProvider';
Expand Down Expand Up @@ -34,4 +36,19 @@ describe('withTheme', () => {
);
assert.strictEqual(wrapper.text(), 'foo');
});

it('does not hoist statics', () => {
const Test = () => null;
Test.someStatic = 'will not get hoisted';
const TestWithTheme = withTheme()(Test);
assert.strictEqual(TestWithTheme.someStatic, undefined);
});

it('hoists mui internals', () => {
assert.strictEqual(isMuiElement(<Input />, ['Input']), true);

const ThemedInput = withTheme()(Input);

assert.strictEqual(isMuiElement(<ThemedInput />, ['Input']), true);
});
});