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

Sewing-kit bump #2369

Merged
merged 6 commits into from Nov 8, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 24 additions & 2 deletions .eslintrc
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
@@ -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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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