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
9 changes: 6 additions & 3 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"allowBlockStart": false
}
],
"babel/no-unused-expressions": "off",
"@babel/no-unused-expressions": "off",
"import/named": "off",
"import/no-default-export": ["error"],
"react/button-has-type": "off",
Expand Down Expand Up @@ -74,7 +74,10 @@
"files": ["**/*.test.{ts,tsx}"],
"rules": {
"jest/no-truthy-falsy": "off",
"@shopify/no-ancestor-directory-import": "off"
"react/jsx-no-constructed-context-values": "off",
"@shopify/jsx-no-hardcoded-content": "off",
"@shopify/no-ancestor-directory-import": "off",
"@shopify/react-require-autocomplete": "off"
}
},
{
Expand All @@ -87,7 +90,7 @@
}
},
{
"files": ["playground/Playground.tsx"],
"files": ["playground/*.tsx"],
"rules": {
"react/prefer-stateless-function": "off",
"@shopify/jsx-no-hardcoded-content": "off",
Expand Down
1 change: 1 addition & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t
- Bumped `node-sass` to `v6.0.1` ([#4783](https://github.com/Shopify/polaris-react/pull/4783))
- Bumped `sass-loader` to `v10.1.1` ([#4783](https://github.com/Shopify/polaris-react/pull/4783))
- Bumped `stylelint` to `v14.1.0` and `@shopify/stylelint-plugin` to `v11.0.0` ([#4798](https://github.com/Shopify/polaris-react/pull/4798))
- Bumped `eslint` to `v8.3.0` and `@shopify/eslint-plugin` to `v41.0.1` ([#4797](https://github.com/Shopify/polaris-react/pull/4797))

### Code quality

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"@rollup/pluginutils": "^3.1.0",
"@shopify/babel-preset": "^24.1.2",
"@shopify/browserslist-config": "^3.0.0",
"@shopify/eslint-plugin": "^39.0.1",
"@shopify/eslint-plugin": "^41.0.1",
"@shopify/jest-dom-mocks": "^3.0.5",
"@shopify/loom": "^1.0.0",
"@shopify/loom-cli": "^1.0.0",
Expand Down Expand Up @@ -121,7 +121,7 @@
"core-js": "^3.6.5",
"create-file-webpack": "^1.0.2",
"downlevel-dts": "^0.6.0",
"eslint": "^7.32.0",
"eslint": "^8.3.0",
"fs-extra": "^7.0.1",
"glob": "^7.1.4",
"gray-matter": "^4.0.2",
Expand Down
4 changes: 3 additions & 1 deletion playground/KitchenSink.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';

type ReadmeModule = Record<string, any>;
interface ReadmeModule {
[key: string]: any;
}

const readmeReq = require.context(
'../src/components',
Expand Down
4 changes: 2 additions & 2 deletions src/components/AppProvider/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ export class AppProvider extends Component<AppProviderProps, State> {
}

render() {
const {theme = {}, children} = this.props;
const {theme = {}, features = {}, children} = this.props;

const {intl, link} = this.state;

return (
<FeaturesContext.Provider value={this.props.features || {}}>
<FeaturesContext.Provider value={features}>
<I18nContext.Provider value={intl}>
<ScrollLockManagerContext.Provider value={this.scrollLockManager}>
<StickyManagerContext.Provider value={this.stickyManager}>
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 @@ -73,8 +73,8 @@ export const Autocomplete: React.FunctionComponent<AutocompleteProps> & {
(options: OptionDescriptor[]) => {
return options.map((option) => (
<MappedOption
{...option}
key={option.id || option.value}
{...option}
selected={selected.includes(option.value)}
singleSelection={!allowMultiple}
/>
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 @@ -333,7 +333,7 @@ describe('<Banner />', () => {
DiamondAlertMajor,
],
])(
'Sets Icon props when: %s',
'sets Icon props when: %s',
(_: any, status: any, color: any, iconSource: any) => {
const banner = mountWithApp(<Banner status={status} />);

Expand Down
2 changes: 1 addition & 1 deletion src/components/BulkActions/BulkActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,9 @@ class BulkActionsInner extends PureComponent<CombinedProps, State> {
}
return (
<BulkActionButton
key={index}
disabled={disabled}
{...action}
key={index}
handleMeasurement={this.handleMeasurement}
/>
);
Expand Down
1 change: 1 addition & 0 deletions src/components/Button/tests/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ describe('<Button />', () => {
});

it.todo('renders a placeholder disclosure icon');

it.todo('renders a placeholder inner icon');
});

Expand Down
5 changes: 3 additions & 2 deletions src/components/DataTable/components/Cell/tests/Cell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ describe('<Cell />', () => {
});
});
});

describe('when set to ascending', () => {
it('renders an up caret Icon when table is not currently sorted by that column', () => {
const cell = mountWithTable(
Expand Down Expand Up @@ -260,8 +261,8 @@ describe('<Cell />', () => {
});
});

function mountWithTable<P>(node: ReactElement) {
return mountWithApp<P>(
function mountWithTable<T>(node: ReactElement) {
return mountWithApp<T>(
<table>
<thead />
<tbody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('<FileUpload />', () => {
measuring: false,
allowMultiple: true,
};

describe('measuring', () => {
it('hides the FileUpload while measuring is true', () => {
const fileUpload = mountWithApp(
Expand Down
4 changes: 3 additions & 1 deletion src/components/DropZone/tests/DropZone.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ describe('<DropZone />', () => {

describe('overlayText', () => {
const overlayText = 'overlay text';

it('does not render the overlayText on small screens', () => {
setBoundingClientRect('small');
const dropZone = mountWithApp(<DropZone overlayText={overlayText} />);
Expand Down Expand Up @@ -351,6 +352,7 @@ describe('<DropZone />', () => {

describe('errorOverlayText', () => {
const errorOverlayText = "can't drop this";

it("doesn't render the overlayText on small screens", () => {
setBoundingClientRect('small');
const dropZone = mountWithApp(
Expand Down Expand Up @@ -552,7 +554,7 @@ function fireEvent({
wrapper: CustomRoot<any, any>;
eventType?: string;
spy?: jest.Mock;
testFiles?: Record<string, unknown>[];
testFiles?: {[key: string]: unknown}[];
}) {
act(() => {
if (spy) {
Expand Down
4 changes: 3 additions & 1 deletion src/components/Frame/Frame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ class FrameInner extends PureComponent<CombinedProps, State> {
classNames={navTransitionClasses}
>
<div
key="NavContent"
{...mobileNavAttributes}
aria-label={i18n.translate('Polaris.Frame.navigationLabel')}
ref={this.navigationNode}
className={navClassName}
onKeyDown={this.handleNavKeydown}
id={APP_FRAME_NAV}
key="NavContent"
hidden={mobileNavHidden}
>
{navigation}
Expand Down Expand Up @@ -243,6 +243,8 @@ class FrameInner extends PureComponent<CombinedProps, State> {
/>
) : null;

// This is probably a legit error but I don't have the time to refactor this
// eslint-disable-next-line react/jsx-no-constructed-context-values
const context = {
showToast: this.showToast,
hideToast: this.hideToast,
Expand Down
1 change: 1 addition & 0 deletions src/components/Frame/tests/Frame.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('<Frame />', () => {
);
});
});

describe('topBar', () => {
it('renders with a top bar data attribute if a topBar is passed', () => {
const topbar = <div />;
Expand Down
4 changes: 2 additions & 2 deletions src/components/IndexTable/tests/IndexTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ function Component({
}

const mockRenderRow = (item: any) => {
return <Component {...item} key={item.id} />;
return <Component key={item.id} {...item} />;
};

const mockRenderCondensedRow = (item: any) => {
return <Component {...item} key={item.id} condensed />;
return <Component key={item.id} {...item} condensed />;
};

describe('<IndexTable>', () => {
Expand Down
9 changes: 7 additions & 2 deletions src/components/MediaQueryProvider/MediaQueryProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect, useState, useCallback} from 'react';
import React, {useEffect, useState, useCallback, useMemo} from 'react';
import debounce from 'lodash/debounce';

import {MediaQueryContext} from '../../utilities/media-query';
Expand Down Expand Up @@ -34,8 +34,13 @@ export const MediaQueryProvider = function MediaQueryProvider({
setIsNavigationCollapsed(navigationBarCollapsed().matches);
}, []);

const context = useMemo(
() => ({isNavigationCollapsed}),
[isNavigationCollapsed],
);

return (
<MediaQueryContext.Provider value={{isNavigationCollapsed}}>
<MediaQueryContext.Provider value={context}>
<EventListener event="resize" handler={handleResize} />
{children}
</MediaQueryContext.Provider>
Expand Down
10 changes: 5 additions & 5 deletions src/components/Navigation/Navigation.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {useMemo} from 'react';

import {Scrollable} from '../Scrollable';
import {useTheme} from '../../utilities/theme';
Expand Down Expand Up @@ -56,10 +56,10 @@ export const Navigation: React.FunctionComponent<NavigationProps> & {
logoMarkup
);

const context = {
location,
onNavigationDismiss: onDismiss,
};
const context = useMemo(
() => ({location, onNavigationDismiss: onDismiss}),
[location, onDismiss],
);

return (
<NavigationContext.Provider value={context}>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Navigation/components/Item/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ export function Item({

return (
<Item
{...rest}
key={label}
{...rest}
label={label}
matches={item === longestMatch}
onClick={onClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ describe('<Nav.Item />', () => {

describe('small screens', () => {
let matchMedia: jest.SpyInstance;

beforeEach(() => {
matchMedia = jest.spyOn(window, 'matchMedia');
matchMedia.mockImplementation(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Navigation/components/Section/Section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ export function Section({

return (
<Item
{...rest}
key={label}
{...rest}
label={label}
subNavigationItems={subNavigationItems}
onClick={handleClick(onClick, hasSubNavItems)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/OptionList/OptionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ export function OptionList({

return (
<Option
{...option}
key={optionId}
{...option}
id={optionId}
section={sectionIndex}
index={optionIndex}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('<Title />', () => {
...mockProps,
titleMetadata: <Badge>Sold</Badge>,
};

it('renders the titleMetadata when defined', () => {
const pageTitle = mountWithApp(<Title {...propsWithMetadata} />);
expect(pageTitle).toContainReactComponent(Badge);
Expand Down
13 changes: 8 additions & 5 deletions src/components/PolarisTestProvider/PolarisTestProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {Fragment, StrictMode} from 'react';
import React, {useMemo, Fragment, StrictMode} from 'react';

import {PortalsManager} from '../PortalsManager';
import {FocusManager} from '../FocusManager';
Expand Down Expand Up @@ -70,12 +70,15 @@ export function PolarisTestProvider({
frame,
}: PolarisTestProviderProps) {
const Wrapper = strict ? StrictMode : Fragment;
const intl = new I18n(i18n || {});
const scrollLockManager = new ScrollLockManager();
const intl = useMemo(() => new I18n(i18n || {}), [i18n]);
Copy link
Member

Choose a reason for hiding this comment

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

Necessary or optimizing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eslint warns with react/jsx-no-constructed-context-values when you sick something in a context that shall cause the context to be updated on every single rerender regardless of if the content changes or not. With the fix being to memoize objects you pass into contexts.

In this particular case it's probably not a big deal as we probably aren't causing rerenders on PolarisTestProvider because, but it's an easy fix to write code in the way to keep the linter happy rather than sprinkling eslint-disable-next-lines around

const scrollLockManager = useMemo(() => new ScrollLockManager(), []);

const stickyManager = new StickyManager();
const stickyManager = useMemo(() => new StickyManager(), []);

const uniqueIdFactory = new UniqueIdFactory(globalIdGeneratorFactory);
const uniqueIdFactory = useMemo(
() => new UniqueIdFactory(globalIdGeneratorFactory),
[],
);

const processedThemeConfig = {...theme, colorScheme: 'light' as const};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('<Section />', () => {
<p>Content</p>
</TextContainer>
);

it('renders its children', () => {
const section = mountWithApp(<Section>{children}</Section>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {setActivatorAttributes} from '../set-activator-attributes';

describe('setActivatorAttributes', () => {
const id = 'id';

it('applies aria-controls to the activator', () => {
const div = document.createElement('div');
setActivatorAttributes(div, {
Expand Down
2 changes: 2 additions & 0 deletions src/components/ProgressBar/ProgressBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function ProgressBar({
);
const parsedProgress = parseProgress(progress, warningMessage);

/* eslint-disable @shopify/jsx-no-hardcoded-content */
return (
<div className={className}>
<progress className={styles.Progress} value={parsedProgress} max="100" />
Expand All @@ -63,6 +64,7 @@ export function ProgressBar({
<span className={styles.Label}>{parsedProgress}%</span>
</div>
</div>
/* eslint-enable @shopify/jsx-no-hardcoded-content */
);
}

Expand Down
1 change: 1 addition & 0 deletions src/components/ProgressBar/tests/ProgressBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('<ProgressBar />', () => {
className: 'Indicator Animated',
});
});

it('sets the progress bar to exclude the Animated class when animated is false', () => {
const progress = mountWithApp(
<ProgressBar animated={false} progress={20} />,
Expand Down
2 changes: 1 addition & 1 deletion src/components/RadioButton/tests/RadioButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('<RadioButton />', () => {
.find('input')!
.prop('id');
expect(typeof id).toBe('string');
expect(id).toBeTruthy();
expect(id).toBeDefined();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export function SingleThumb(props: SingleThumbProps) {
disabled && styles.disabled,
);

/* eslint-disable @shopify/react-require-autocomplete */
return (
<Labelled
id={id}
Expand Down Expand Up @@ -116,6 +117,7 @@ export function SingleThumb(props: SingleThumbProps) {
</div>
</Labelled>
);
/* eslint-enable @shopify/react-require-autocomplete */

function handleChange(event: React.ChangeEvent<HTMLInputElement>) {
const {onChange} = props;
Expand Down
Loading