Skip to content
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
26 changes: 24 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,36 @@
"jsx-a11y/mouse-events-have-key-events": "off",
"jsx-a11y/click-events-have-key-events": "off",
"jsx-a11y/no-noninteractive-element-interactions": "off",
"jsx-a11y/no-noninteractive-element-to-interactive-role": "off"
"jsx-a11y/no-noninteractive-element-to-interactive-role": "off",
"@typescript-eslint/camelcase": [
"error",
{
"allow": [
"UNSTABLE_Color",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit nicer than sprinkling single-line ignores throughout the codebase

"UNSTABLE_colors",
"UNSTABLE_cssCustomProperties",
"UNSTABLE_telemetry"
]
}
]
},
"overrides": [
{
"files": ["src/**/*.{ts,tsx}"],
Copy link
Member Author

Choose a reason for hiding this comment

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

Because some of our ts files aren't included in our tsconfig's files list we can't set this stuff at the global level, which is a little annoying but not the end of the world.

"extends": ["plugin:shopify/typescript-type-checking"],
"parserOptions": {
"project": "./tsconfig.json"
},
"rules": {
"@typescript-eslint/prefer-readonly": "off",
"@typescript-eslint/no-unnecessary-condition": "off",
"@typescript-eslint/unbound-method": "off"
}
},
{
"files": ["**/*.test.{ts,tsx}"],
"rules": {
"jest/no-truthy-falsy": "off",
"shopify/jsx-no-hardcoded-content": "off",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now turned off in tests by default

"shopify/no-ancestor-directory-import": "off"
}
},
Expand Down
11 changes: 6 additions & 5 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
// Place your settings in this file to overwrite default and user settings.
{
"css.validate": false,
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.formatOnSave": true,
"eslint.autoFixOnSave": true,
"eslint.validate": [
"javascript",
"javascriptreact",
"typescript",
"typescriptreact"
{"language": "javascript", "autoFix": true},
{"language": "javascriptreact", "autoFix": true},
{"language": "typescript", "autoFix": true},
{"language": "typescriptreact", "autoFix": true}
],
"files.exclude": {
"**/.DS_Store": true,
Expand All @@ -27,7 +29,6 @@
},
"javascript.validate.enable": false,
"jest.autoEnable": false,
"prettier.eslintIntegration": true,
"prettier.stylelintIntegration": true,
"scss.validate": false,
"search.exclude": {
Expand Down
1 change: 1 addition & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
### Development workflow

- Enabled maintainers running `yarn dev` to hide [`yarn splash`](https://github.com/Shopify/polaris-react/tree/master/scripts/splash) reports from the console by running `DISABLE_SPLASH=1 yarn dev` ([#2372](https://github.com/Shopify/polaris-react/pull/2372))
- Updated to sewing-kit 0.112.0 and eslint 6 and updated vscode config to use the eslint plugin to format js/ts files ((#2369)[https://github.com/Shopify/polaris-react/pull/2369])

### Dependency upgrades

Expand Down
5 changes: 1 addition & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
"@percy/storybook": "^3.2.0",
"@shopify/jest-dom-mocks": "^2.1.1",
"@shopify/react-testing": "^1.7.8",
"@shopify/sewing-kit": "^0.111.0",
"@shopify/sewing-kit": "^0.112.0",
"@storybook/addon-a11y": "^5.2.4",
"@storybook/addon-actions": "^5.2.4",
"@storybook/addon-console": "^1.2.1",
Expand Down Expand Up @@ -162,9 +162,6 @@
"react": "^16.8.6",
"react-dom": "^16.8.6"
},
"resolutions": {
"typescript-eslint-parser": "npm:@typescript-eslint/parser@1.10.2"
},
"files": [
"esnext",
"styles",
Expand Down
1 change: 1 addition & 0 deletions scripts/pa11y.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ async function runPa11y() {
];

await browsers.forEach(async (instance) => {
// eslint-disable-next-line require-atomic-updates
instance.page = await instance.browser.newPage();
});

Expand Down
6 changes: 3 additions & 3 deletions scripts/splash/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const Components = ({components, status}) => (
<React.Fragment>
{status === 'loading' && (
<Box marginLeft={4} marginBottom={1}>
{' '}Please wait during compilation… Beep boop beep 🤖
Please wait during compilation… Beep boop beep 🤖
Copy link
Contributor

Choose a reason for hiding this comment

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

No regression here – all good.

</Box>
)}

Expand Down Expand Up @@ -202,8 +202,8 @@ const App = () => {
<Color dim>
<Box width={3}>💡</Box>
<Box>
Tip: to disable these reports, run{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes a regression where a space is missing between run and DISABLE_SPLASH:

Screen Shot 2019-11-08 at 3 20 51 PM

<Text bold>DISABLE_SPLASH=1 yarn dev</Text>
Tip: to disable these reports, run
<Text bold> DISABLE_SPLASH=1 yarn dev</Text>
</Box>
</Color>
</Box>
Expand Down
8 changes: 4 additions & 4 deletions scripts/splash/treebuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import * as ts from 'typescript';
import glob from 'glob';
import cmd from 'node-cmd';

type Node = {
interface Node {
fileName: string;
dependsOn: Node[];
dependedOnBy: Node[];
};
}

type GraphType = {
interface GraphType {
[name: string]: Node;
};
}

const graph: GraphType = {};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('<AccountConnection />', () => {
it('is shown on the card when provided', () => {
const TermsOfService = () => (
<p>
By clicking <strong>Connect</strong>, you agree to accept Sample App’s{' '}
By clicking <strong>Connect</strong>, you agree to accept Sample App’s
terms and conditions. You’ll pay a commission rate of 15% on sales
made through Sample App.
</p>
Expand Down
3 changes: 0 additions & 3 deletions src/components/AppProvider/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export interface AppProviderProps extends AppBridgeOptions {
features?: Features;
/** Inner content of the application */
children?: React.ReactNode;
// eslint-disable-next-line babel/camelcase
UNSTABLE_telemetry?: TelemetryObject;
}

Expand Down Expand Up @@ -100,7 +99,6 @@ export class AppProvider extends React.Component<AppProviderProps, State> {
});
}

/* eslint-disable babel/camelcase */
render() {
const {
theme = {},
Expand Down Expand Up @@ -132,5 +130,4 @@ export class AppProvider extends React.Component<AppProviderProps, State> {
</FeaturesContext.Provider>
);
}
/* eslint-enable babel/camelcase */
}
2 changes: 1 addition & 1 deletion src/components/AppProvider/tests/AppProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('<AppProvider />', () => {
});

it('updates context when props change', () => {
const Child: React.SFC<{}> = () => {
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 lot of the changes in this PR are "that's the default value for the generic, you don't need to specify that"

const Child: React.SFC = () => {
return useContext(LinkContext) ? <div id="child" /> : null;
};
const LinkComponent = () => <div />;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Autocomplete/Autocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface AutocompleteProps {
/** The selected options */
selected: string[];
/** The text field component attached to the list of options */
textField: React.ReactElement<any>;
textField: React.ReactElement;
/** The preferred direction to open the popover */
preferredPosition?: PreferredPosition;
/** Title of the list of options */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface ComboBoxProps {
/** The selected options */
selected: string[];
/** The text field component attached to the list of options */
textField: React.ReactElement<any>;
textField: React.ReactElement;
/** The preferred direction to open the popover */
preferredPosition?: PreferredPosition;
/** Title of the list of options */
Expand Down
2 changes: 1 addition & 1 deletion src/components/Banner/tests/Banner.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('<Banner />', () => {

describe('context', () => {
it('passes the within banner context', () => {
const Child: React.SFC<{}> = (_props) => {
const Child: React.SFC = (_props) => {
return (
<BannerContext.Consumer>
{(BannerContext) => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import styles from './Breadcrumbs.scss';

export interface BreadcrumbsProps {
/** Collection of breadcrumbs */
breadcrumbs: Array<CallbackAction | LinkAction>;
breadcrumbs: (CallbackAction | LinkAction)[];
}

export class Breadcrumbs extends React.PureComponent<BreadcrumbsProps, never> {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Choice/Choice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface ChoiceProps {
/** Label for the choice */
label: React.ReactNode;
/** Whether the associated form control is disabled */
disabled?: Boolean;
disabled?: boolean;
/** Display an error message */
error?: Error | boolean;
/** Visually hide the label */
Expand Down
4 changes: 2 additions & 2 deletions src/components/Choice/tests/Choice.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ describe('<Choice />', () => {
<Choice id="MyChoice" label="Label" />,
);
const label = element.find('label');
for (let i = 0; i < blockLevelElements.length; i++) {
expect(label.find(blockLevelElements[i])).toHaveLength(0);
for (const blockLevelElement of blockLevelElements) {
expect(label.find(blockLevelElement)).toHaveLength(0);
}
});
});
2 changes: 1 addition & 1 deletion src/components/ChoiceList/ChoiceList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export function ChoiceList({
function noop() {}

function choiceIsSelected({value}: Choice, selected: string[]) {
return selected.indexOf(value) >= 0;
return selected.includes(value);
}

function updateSelectedChoices(
Expand Down
20 changes: 8 additions & 12 deletions src/components/Connected/tests/Connected.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,18 @@ import {Item, ItemPosition} from '../components';

describe('<Connected />', () => {
describe('<Item />', () => {
it('wraps children in an Item component', async () => {
it('wraps children in an Item component', () => {
const expectedContent = 'foo';
const connected = await mountWithApp(
<Connected>{expectedContent}</Connected>,
);
const connected = mountWithApp(<Connected>{expectedContent}</Connected>);

expect(
connected.find(Item, {position: ItemPosition.Primary}),
).toContainReactText(expectedContent);
});

it('includes `rightConnected` markup in an Item component', async () => {
it('includes `rightConnected` markup in an Item component', () => {
const rightConnectedContent = 'foo';
const connected = await mountWithApp(
const connected = mountWithApp(
<Connected right={rightConnectedContent} />,
);

Expand All @@ -28,21 +26,19 @@ describe('<Connected />', () => {
).toContainReactText(rightConnectedContent);
});

it('includes `leftConnected` markup in an Item component', async () => {
it('includes `leftConnected` markup in an Item component', () => {
const leftConnectedContent = 'foo';
const connected = await mountWithApp(
<Connected left={leftConnectedContent} />,
);
const connected = mountWithApp(<Connected left={leftConnectedContent} />);

expect(
connected.find(Item, {position: ItemPosition.Left}),
).toContainReactText(leftConnectedContent);
});

it('`leftConnected` and `rightConnected` are not mutually exclusive', async () => {
it('`leftConnected` and `rightConnected` are not mutually exclusive', () => {
const rightConnectedContent = 'rightfoo';
const leftConnectedContent = 'leftfoo';
const connected = await mountWithApp(
const connected = mountWithApp(
<Connected right={rightConnectedContent} left={leftConnectedContent} />,
);

Expand Down
3 changes: 0 additions & 3 deletions src/components/ContextualSaveBar/ContextualSaveBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import {ContextualSaveBarProps, useFrame} from '../../utilities/frame';
// crashing if we write `ContextualSaveBar extends React.Component<ContextualSaveBarProps>`
export interface ContextualSaveBarProps extends ContextualSaveBarProps {}

// This does have a display name, but the linting has a bug in it
// https://github.com/yannickcr/eslint-plugin-react/issues/2324
// eslint-disable-next-line react/display-name
export const ContextualSaveBar = React.memo(function ContextualSaveBar({
message,
saveAction,
Expand Down
4 changes: 2 additions & 2 deletions src/components/DatePicker/components/Month/Month.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface MonthProps {
year: Year;
disableDatesBefore?: Date;
disableDatesAfter?: Date;
allowRange?: Boolean;
allowRange?: boolean;
weekStartsOn: Weekdays;
onChange?(date: Range): void;
onHover?(hoverEnd: Date): void;
Expand Down Expand Up @@ -82,7 +82,7 @@ export function Month({
));

function handleDateClick(selectedDate: Date) {
onChange(getNewRange(allowRange && selected, selectedDate));
onChange(getNewRange(allowRange ? selected : undefined, selectedDate));
}

function renderWeek(day: Date, dayIndex: number) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/DropZone/DropZone.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ class DropZone extends React.Component<CombinedProps, State> {

const fileList = getDataTransferFiles(event);

if (event.target && this.dragTargets.indexOf(event.target) === -1) {
if (event.target && !this.dragTargets.includes(event.target)) {
this.dragTargets.push(event.target);
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/DropZone/tests/DropZone.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ function fireEvent({
element: ReactWrapper<any, any>;
eventType?: string;
spy?: jest.Mock;
testFiles?: Array<Object>;
testFiles?: object[];
}) {
if (spy) {
spy.mockReset();
Expand Down
4 changes: 2 additions & 2 deletions src/components/DropZone/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ function accepts(file: File, acceptedFiles: string | string[] | undefined) {

return acceptedFilesArray.some((type) => {
const validType = type.trim();
if (validType.charAt(0) === '.') {
if (validType.startsWith('.')) {
return fileName.toLowerCase().endsWith(validType.toLowerCase());
} else if (/\/\*$/.test(validType)) {
} else if (validType.endsWith('/*')) {
// This is something like a image/* mime type
return baseMimeType === validType.replace(/\/.*$/, '');
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/ExceptionList/ExceptionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import styles from './ExceptionList.scss';

export type Description =
| string
| React.ReactElement<any>
| (string | React.ReactElement<any>)[];
| React.ReactElement
| (string | React.ReactElement)[];

export interface Item {
/** Set the color of the icon and title for the given item. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,7 @@ export class ConnectedFilterControl extends React.Component<
return actionsToReturn;
}

private activatorButtonFrom(
action: PopoverableAction,
): React.ReactElement<any> {
private activatorButtonFrom(action: PopoverableAction): React.ReactElement {
return (
<Button
onClick={action.onAction}
Expand All @@ -200,7 +198,7 @@ export class ConnectedFilterControl extends React.Component<
);
}

private popoverFrom(actions: PopoverableAction[]): React.ReactElement<any>[] {
private popoverFrom(actions: PopoverableAction[]): React.ReactElement[] {
return actions.map((action) => {
return (
<Item key={action.key}>
Expand Down
3 changes: 0 additions & 3 deletions src/components/Frame/components/ToastManager/ToastManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ export interface ToastManagerProps {
toastMessages: (ToastPropsWithID)[];
}

// This does have a display name, but the linting has a bug in it
// https://github.com/yannickcr/eslint-plugin-react/issues/2324
// eslint-disable-next-line react/display-name
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 🔥

export const ToastManager = memo(function ToastManager({
toastMessages,
}: ToastManagerProps) {
Expand Down
Loading