Skip to content

Dependency updates #2326

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

Merged
merged 3 commits into from
Oct 18, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
],
"settings": {
"react": {
"version": "16.3"
"version": "16.8"
}
},
"rules": {
"func-style": "off",
"no-process-env": "off",
"no-warning-comments": "off",
"no-negated-condition": "off",
"no-console": "error",
"consistent-return": "off",
"match-default-export-name": "off",
"jsx-use-translation-function": "off",
Expand Down
2 changes: 2 additions & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f

### Dependency upgrades

- Updated sewing-kit to v0.111.0 and storybook to v5.2.4 ([#2326](https://github.com/Shopify/polaris-react/pull/2326))

### Code quality

### Deprecations
Expand Down
16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@
"@percy/storybook": "^3.2.0",
"@shopify/jest-dom-mocks": "^2.1.1",
"@shopify/react-testing": "^1.7.8",
"@shopify/sewing-kit": "^0.92.3",
"@storybook/addon-a11y": "^5.2.1",
"@storybook/addon-actions": "^5.2.1",
"@shopify/sewing-kit": "^0.111.0",
"@storybook/addon-a11y": "^5.2.4",
"@storybook/addon-actions": "^5.2.4",
"@storybook/addon-console": "^1.2.1",
"@storybook/addon-contexts": "^5.2.1",
"@storybook/addon-notes": "^5.2.1",
"@storybook/addon-viewport": "^5.2.1",
"@storybook/react": "^5.2.1",
"@storybook/theming": "^5.2.1",
"@storybook/addon-contexts": "^5.2.4",
"@storybook/addon-notes": "^5.2.4",
"@storybook/addon-viewport": "^5.2.4",
"@storybook/react": "^5.2.4",
"@storybook/theming": "^5.2.4",
"@types/enzyme": "^3.10.3",
"@types/enzyme-adapter-react-16": "^1.0.5",
"@types/lodash": "^4.14.138",
Expand Down
3 changes: 3 additions & 0 deletions scripts/new-version-pr-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ The automatic branch creation for ${repository} failed. This can be due to many
mkdir(sandbox);

const jobs = repositories.map((repository) => {
// TODO work out how to keep eslint happy
// SEE https://eslint.org/docs/rules/no-async-promise-executor
// eslint-disable-next-line no-async-promise-executor
return new Promise(async (resolve, reject) => {
try {
const repositoryDirectory = path.resolve(sandbox, repository);
Expand Down
3 changes: 3 additions & 0 deletions scripts/utilities/retry.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ function sleep(ms) {
}

function retry(functionToTry, maxAttempts = 3, delay = 1000) {
// TODO work out how to keep eslint happy
// SEE https://eslint.org/docs/rules/no-async-promise-executor
// eslint-disable-next-line no-async-promise-executor
return new Promise(async (resolve, reject) => {
let remainingAttempts = maxAttempts;
while (remainingAttempts > 0) {
Expand Down
3 changes: 0 additions & 3 deletions sewing-kit.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ export default function sewingKitConfig(
plugins.jest((config: InitialOptions) => {
config.roots = [join(__dirname, 'src'), join(__dirname, 'tests')];

config.setupFiles.push(join(tests, 'setup.ts'));
Copy link
Member Author

Choose a reason for hiding this comment

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

Sewing-kit now automatically adds these files to the setupFiles and setupFilesAfterEnv blocks if they exist, so we don't need to be explict here.

config.setupFilesAfterEnv = [join(tests, 'each-test.ts')];

// Code coverage
config.collectCoverageFrom = [
'src/**/*.{ts,tsx}',
Expand Down
1 change: 0 additions & 1 deletion src/components/AppProvider/tests/AppProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ describe('<AppProvider />', () => {

it('updates context when props change', () => {
const Child: React.SFC<{}> = () => {
// eslint-disable-next-line shopify/jest/no-if
return useContext(LinkContext) ? <div id="child" /> : null;
};
const LinkComponent = () => <div />;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Autocomplete/tests/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import {CirclePlusMinor} from '@shopify/polaris-icons';
import {mountWithAppProvider, trigger} from 'test-utilities/legacy';
import {Spinner} from 'components';
import {Key} from '../../../types';
import {Autocomplete} from '..';
import {ComboBox} from '../components';
import {Autocomplete} from '..';

describe('<Autocomplete/>', () => {
const options = [
Expand Down
3 changes: 1 addition & 2 deletions src/components/Banner/tests/Banner.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {ReactWrapper} from 'enzyme';
import {mountWithAppProvider} from 'test-utilities/legacy';
import {BannerContext} from 'utilities/banner-context';
import {Button, Icon, UnstyledLink, Heading} from 'components';
import {Banner} from '..';
import {WithinContentContext} from '../../../utilities/within-content-context';
import {Banner} from '..';

describe('<Banner />', () => {
it('renders a title', () => {
Expand Down Expand Up @@ -156,7 +156,6 @@ describe('<Banner />', () => {
return (
<BannerContext.Consumer>
{(BannerContext) => {
// eslint-disable-next-line shopify/jest/no-if
return BannerContext ? <div /> : null;
}}
</BannerContext.Consumer>
Expand Down
1 change: 0 additions & 1 deletion src/components/DropZone/tests/DropZone.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ describe('<DropZone />', () => {
<DropZone type="image">
<DropZoneContext.Consumer>
{(ctx) => {
// eslint-disable-next-line shopify/jest/no-if
return type === ctx.type ? <div /> : null;
}}
</DropZoneContext.Consumer>
Expand Down
2 changes: 1 addition & 1 deletion src/components/DropZone/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,5 @@ function isDragEvent(event: DropZoneEvent): event is DragEvent {
function isChangeEvent(
event: DropZoneEvent,
): event is React.ChangeEvent<HTMLInputElement> {
return event.hasOwnProperty('target');
return Object.prototype.hasOwnProperty.call(event, 'target');
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ describe('MediaQueryProvider', () => {
it('passes isNavigationCollapsed to MediaQueryContext.Provider', () => {
function Component() {
const mediaQuery = useMediaQuery();
// eslint-disable-next-line shopify/jest/no-if
return mediaQuery !== undefined ? <div /> : null;
}

Expand All @@ -40,7 +39,6 @@ describe('MediaQueryProvider', () => {
it('sets isNavigationCollapsed when resize occurs', () => {
function Component() {
const {isNavigationCollapsed} = useMediaQuery();
// eslint-disable-next-line shopify/jest/no-if
return isNavigationCollapsed ? <div>content</div> : null;
}
const mediaQueryProvider = mountWithApp(
Expand Down
2 changes: 0 additions & 2 deletions src/components/Navigation/tests/Navigation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('<Navigation />', () => {
return (
<NavigationContext.Consumer>
{({location}) => {
// eslint-disable-next-line shopify/jest/no-if
return location ? <div /> : null;
}}
</NavigationContext.Consumer>
Expand Down Expand Up @@ -44,7 +43,6 @@ describe('<Navigation />', () => {
return (
<WithinContentContext.Consumer>
{(withinContentContainer) => {
// eslint-disable-next-line shopify/jest/no-if
return withinContentContainer ? <div /> : null;
}}
</WithinContentContext.Consumer>
Expand Down
5 changes: 4 additions & 1 deletion src/components/OptionList/OptionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ function createNormalizedOptions(
}

function isSection(arr: Descriptor[]): arr is SectionDescriptor[] {
return typeof arr[0] === 'object' && arr[0].hasOwnProperty('options');
return (
typeof arr[0] === 'object' &&
Object.prototype.hasOwnProperty.call(arr[0], 'options')
);
}

function optionArraysAreEqual(
Expand Down
2 changes: 1 addition & 1 deletion src/components/PageActions/tests/PageActions.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React from 'react';
import {mountWithAppProvider} from 'test-utilities/legacy';
import {PageActions} from '..';
import {ButtonGroup} from '../../ButtonGroup';
import {Stack} from '../../Stack';
import {buttonsFrom} from '../../Button';
import {PageActions} from '..';

jest.mock('../../Button', () => ({
...require.requireActual('../../Button'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ describe('PolarisTestProvider', () => {
it('allows isNavigationCollapsed to be overwritten', () => {
function Component() {
const {isNavigationCollapsed} = useMediaQuery();
// eslint-disable-next-line shopify/jest/no-if
return isNavigationCollapsed ? <div /> : null;
}

Expand Down
4 changes: 3 additions & 1 deletion src/components/ResourceList/ResourceList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,9 @@ function getAllItemsOnPage(
}

function defaultIdForItem(item: any, index: number) {
return item.hasOwnProperty('id') ? item.id : index.toString();
return Object.prototype.hasOwnProperty.call(item, 'id')
? item.id
: index.toString();
}

function isSmallScreen() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {CheckableButton} from '../../CheckableButton';
import {BulkActionButton} from '../components';
import BulkActions, {BulkAction} from '../BulkActions';

export interface Props {
interface Props {
bulkActions: BulkAction[];
promotedActions: BulkAction[];
paginatedSelectAllText: string;
Expand All @@ -16,7 +16,7 @@ export interface Props {
disabled: boolean;
}

export type TestValue = BulkAction[] | string | boolean;
type TestValue = BulkAction[] | string | boolean;

const bulkActionProps: Props = {
bulkActions: [
Expand Down
1 change: 0 additions & 1 deletion src/components/ScrollLock/tests/ScrollLock.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('ScrollLock', () => {

const setScollLockFalse = useCallback(() => setScrollLock(false), []);

// eslint-disable-next-line shopify/jest/no-if
const scrollLockMarkup = showScrollLock ? <ScrollLock /> : null;

return (
Expand Down
1 change: 0 additions & 1 deletion src/components/Scrollable/tests/Scrollable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ describe('<Scrollable />', () => {
const Child: React.SFC<{}> = (_) => (
<ScrollableContext.Consumer>
{(scrollToPosition) => {
// eslint-disable-next-line shopify/jest/no-if
return scrollToPosition ? <div /> : null;
}}
</ScrollableContext.Consumer>
Expand Down
1 change: 0 additions & 1 deletion src/components/ThemeProvider/tests/ThemeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('<ThemeProvider />', () => {
return (
<ThemeContext.Consumer>
{(polarisTheme) => {
// eslint-disable-next-line shopify/jest/no-if
return polarisTheme && polarisTheme.logo ? <div /> : null;
}}
</ThemeContext.Consumer>
Expand Down
5 changes: 3 additions & 2 deletions src/components/UnstyledLink/UnstyledLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import {useLink, LinkLikeComponentProps} from '../../utilities/link';
// but the props explorer isn't smart enough to work that out
export interface UnstyledLinkProps extends LinkLikeComponentProps {}

// 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.

Is this needed anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, still needed. We're using eslint-plugin-eslint-comments which contains rules than trigger a warning when we've got a disable line that doesn't do anything, so if this was unneeded it would be flagged.

This issue has been resolved, and is in v7.15.0 but eslint-plugin-shopify depends up on v7.14.3 exactly.

On eslint-plugin-shopify's master branch that dependency has been bumped, so the next time theres a release of eslint-plugin-shopify this can be fixed up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok! Found it a bit strange that the eslint-disable-next-line was moved.

Copy link
Member Author

Choose a reason for hiding this comment

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

There was a version bumb to eslint-plugin-react but not to the version we need - Now it raises a warning on both the React.memo and React.forwardRef - both of which are spurious.

Tbh there's no value in wrapping this in a memo anyway so I'm planning to remove that in a later PR.

export const UnstyledLink = React.memo(
// 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
React.forwardRef<unknown, UnstyledLinkProps>(function UnstyledLink(
props,
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/color-transformers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {compose} from './compose';
export function rgbString(color: RGBColor | RGBAColor) {
const {red, green, blue} = color;

if (color.hasOwnProperty('alpha')) {
if (Object.prototype.hasOwnProperty.call(color, 'alpha')) {
return `rgba(${red}, ${green}, ${blue}, ${(color as RGBAColor).alpha})`;
} else {
return `rgb(${red}, ${green}, ${blue})`;
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/i18n/I18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function translate(
return text.replace(REPLACE_REGEX, (match: string) => {
const replacement: string = match.substring(1, match.length - 1);

if (!replacements.hasOwnProperty(replacement)) {
if (!Object.prototype.hasOwnProperty.call(replacements, replacement)) {
throw new Error(
`No replacement found for key '${replacement}'. The following replacements were passed: ${Object.keys(
replacements,
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function merge<TSource1, TSource2, TSource3, TSource4, TSource5>(

function mergeRecursively(objA: GeneralObject, objB: GeneralObject) {
for (const key in objB) {
if (!objB.hasOwnProperty(key)) {
if (!Object.prototype.hasOwnProperty.call(objB, key)) {
continue;
} else if (isMergeableValue(objB[key]) && isMergeableValue(objA[key])) {
objA[key] = mergeRecursively(objA[key], objB[key]);
Expand Down
7 changes: 5 additions & 2 deletions src/utilities/pick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ function pickValueAndLength(obj: GeneralObject, key: string) {
const keyPaths = key.split('.');
let value = obj;
for (const key of keyPaths) {
if (!value.hasOwnProperty(key)) {
if (!Object.prototype.hasOwnProperty.call(value, key)) {
return null;
}

Expand All @@ -21,7 +21,10 @@ export function pick(
const flattenedKeypaths = ([] as string[]).concat(...keyPaths);
if (obj == null || flattenedKeypaths.length === 0) return {};
return flattenedKeypaths.reduce((acc, key) => {
if (typeof key !== TypeOf.String || obj.hasOwnProperty(key)) {
if (
typeof key !== TypeOf.String ||
Object.prototype.hasOwnProperty.call(obj, key)
) {
return {...acc, [key]: obj[key]};
}

Expand Down
1 change: 0 additions & 1 deletion src/utilities/tests/use-is-after-initial-mount.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ describe('useIsAfterInitialMount', () => {
let isAfterInitialMountValue: boolean | undefined;
function Component() {
const isAfterInitialMount = useIsAfterInitialMount();
// eslint-disable-next-line shopify/jest/no-if
if (isAfterInitialMountValue === undefined)
isAfterInitialMountValue = !isAfterInitialMount;
return null;
Expand Down
1 change: 0 additions & 1 deletion src/utilities/unique-id/tests/hooks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ describe('useUniqueId', () => {
[],
);

// eslint-disable-next-line shopify/jest/no-if
const override = count % 2 === 0 ? `Override${count}` : undefined;

return (
Expand Down
Loading