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] Warn when using a missing style rule #16501

Closed
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 docs/src/modules/components/MarkdownDocs.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function MarkdownDocs(props) {
</Portal>
)}

<AppContent disableToc={disableToc} className={classes.root}>
<AppContent disableToc={disableToc}>
Copy link
Member Author

Choose a reason for hiding this comment

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

A dead class name found with the new warning.

Copy link
Member

Choose a reason for hiding this comment

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

If you're concerned with using missing classes then using types is the better approach.

{!disableEdit ? (
<div className={classes.header}>
<EditPage
Expand Down
16 changes: 16 additions & 0 deletions docs/src/modules/utils/generateMarkdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ function escapeCell(value) {
.replace(/\|/g, '\\|');
}

function isOneOfWithString(type) {
return type.raw.indexOf('oneOfWithString') === 0;
}

function isElementTypeAcceptingRefProp(type) {
return type.raw === 'elementTypeAcceptingRef';
}
Expand Down Expand Up @@ -170,6 +174,18 @@ function generatePropType(type) {
if (isElementAcceptingRefProp(type)) {
return `element`;
}
if (isOneOfWithString(type)) {
return generatePropType({
name: 'enum',
value: type.raw
.replace('oneOfWithString([', '')
.replace('])', '')
.split(',')
.map(x => x.trim())
.filter(x => x)
.map(x => ({ value: x, computed: false })),
});
}

const deprecatedInfo = getDeprecatedInfo(type);
if (deprecatedInfo !== false) {
Expand Down
28 changes: 28 additions & 0 deletions packages/material-ui-styles/src/makeStyles/makeStyles.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import warning from 'warning';
import { getDynamicStyles } from 'jss';
import { getDisplayName } from '@material-ui/utils';
import mergeClasses from '../mergeClasses';
import multiKeyStore from './multiKeyStore';
import useTheme from '../useTheme';
Expand Down Expand Up @@ -44,6 +45,33 @@ function getClasses({ state, stylesOptions }, classes, Component) {
newClasses: classes,
Component,
});

if (process.env.NODE_ENV !== 'production' && typeof Proxy !== 'undefined') {
state.cacheClasses.value = new Proxy(state.cacheClasses.value, {
get(target, styleRule) {
if (
!(styleRule in target) &&
styleRule !== '@@toStringTag' &&
typeof styleRule !== 'symbol' &&
styleRule !== 'inspect'
) {
const name = state.name || getDisplayName(Component);
warning(
false,
[
`Material-UI: you access a style rule \`${styleRule}\` that does not exist.`,
styleRule && `The style was definied on the \`${name}\` component.`,
'',
'Instead, the following classes keys are available:',
JSON.stringify(target, null, 2),
].join('\n'),
);
}

return target[styleRule];
},
});
}
}

return state.cacheClasses.value;
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Typography/Typography.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
import { capitalize } from '../utils/helpers';
import oneOfWithString from '../utils/oneOfWithString';

export const styles = theme => ({
/* Styles applied to the root element. */
Expand Down Expand Up @@ -221,7 +222,7 @@ Typography.propTypes = {
/**
* Applies the theme typography styles.
*/
variant: PropTypes.oneOf([
variant: oneOfWithString([
Copy link
Member

Choose a reason for hiding this comment

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

How does react-docgen handle PropTypes.oneOfType([PropTypes.string, PropTypes.oneOf([...]])

'h1',
'h2',
'h3',
Expand Down
5 changes: 5 additions & 0 deletions packages/material-ui/src/utils/oneOfWithString.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as PropTypes from 'prop-types';

export default function oneOfWithString() {
return PropTypes.string;
}