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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update sewing-kit #532

Merged
merged 4 commits into from Nov 2, 2018
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
17 changes: 12 additions & 5 deletions .eslintrc
@@ -1,10 +1,12 @@
{
"extends": [
"plugin:shopify/typescript-react",
"plugin:shopify/typescript",
"plugin:shopify/react",
"plugin:shopify/eslint-comments",
"plugin:shopify/jest",
"plugin:shopify/node",
"plugin:shopify/polaris",
"plugin:shopify/typescript-prettier"
"plugin:shopify/prettier"
],
"settings": {
"react": {
Expand All @@ -26,9 +28,12 @@
"allowBlockStart": false
}
],
"import/no-cycle": "off",
Copy link
Member Author

@BPScott BPScott Nov 2, 2018

Choose a reason for hiding this comment

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

Initially disabled rules!

  • import/no-cycle shall be enabled in a follow-up PR as this one is already noisy enough as-is, and fixing it will require a lot of changes and there's potential for weirdness as I unpick the cycles.
  • import/no-named-as-default I'm not really sure what to do with. This currently has lots of errors and fixing it may require rethinking how we we write imports/exports. It doesn't help that I think the rule itself might be a bit buggy.
  • shopify/jsx-no-complex-expressions we probably want to keep disabled, This has 10 violations currently and in those cases we can't avoid it without some small rewrites that are more complex than I am comfortable putting in this PR that is mostly brainless changes.

Copy link
Member

Choose a reason for hiding this comment

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

All sounds good 馃憤 I think we might wanna create follow up issues for all of these so we don't forget about them.

"import/no-named-as-default": "off",
"react/button-has-type": "off",
"react/no-array-index-key": "off",
"shopify/jest/no-vague-titles": "off",
"shopify/jsx-no-complex-expressions": "off",
"jsx-a11y/label-has-for": [
2,
{
Expand All @@ -48,15 +53,17 @@
},
"overrides": [
{
"files": ["*.js"],
"files": ["examples/**/*.js"],
"rules": {
"typescript/no-var-requires": "off"
"import/no-extraneous-dependencies": "off",
"import/no-unresolved": "off"
}
},
{
"files": ["playground/Playground.tsx"],
"rules": {
"react/prefer-stateless-function": "off"
"react/prefer-stateless-function": "off",
"shopify/react-initialize-state": "off"
}
}
]
Expand Down
2 changes: 1 addition & 1 deletion config/rollup/plugins/styles.js
@@ -1,6 +1,6 @@
import {resolve as resolvePath, dirname} from 'path';
import postcss from 'postcss';
import {readFileSync, ensureDirSync, writeFile} from 'fs-extra';
import {resolve as resolvePath, dirname} from 'path';
import {render} from 'node-sass';
import {createFilter} from 'rollup-pluginutils';
import cssnano from 'cssnano';
Expand Down
3 changes: 1 addition & 2 deletions package.json
Expand Up @@ -89,7 +89,7 @@
"@shopify/jest-dom-mocks": "^2.0.5",
"@shopify/js-uploader": "github:shopify/js-uploader",
"@shopify/react-serialize": "^1.0.6",
"@shopify/sewing-kit": "^0.61.0",
"@shopify/sewing-kit": "0.62.0",
"@types/enzyme": "^3.1.14",
"@types/enzyme-adapter-react-16": "^1.0.3",
"@types/lodash": "^4.14.108",
Expand Down Expand Up @@ -178,7 +178,6 @@
"react-dom": "^16.3.1"
},
"resolutions": {
"ts-jest": "~23.10.4",
"typescript-eslint-parser": "17.0.1"
},
"files": [
Expand Down
1 change: 0 additions & 1 deletion playground/index.tsx
Expand Up @@ -5,7 +5,6 @@ import {AppContainer} from 'react-hot-loader';
import {AppProvider} from '@shopify/polaris';

function renderPlayground() {
// eslint-disable-next-line no-require-imports
const Playground = require('./Playground').default;
render(
<AppContainer>
Expand Down
1 change: 0 additions & 1 deletion playground/webpack.config.js
Expand Up @@ -12,7 +12,6 @@ module.exports = {
target: 'web',
devtool: 'eval',
devServer: {
// eslint-disable-next-line no-process-env
port: process.env.PORT || 8080,
disableHostCheck: true,
},
Expand Down
2 changes: 1 addition & 1 deletion scripts/build-consumer.js
@@ -1,7 +1,7 @@
/* eslint-disable no-console */

import {cp, mkdir, rm} from 'shelljs';
import {resolve} from 'path';
import {cp, mkdir, rm} from 'shelljs';

import packageJSON from '../package.json';

Expand Down
10 changes: 5 additions & 5 deletions scripts/build-shrink-ray.js
Expand Up @@ -3,12 +3,12 @@
require('isomorphic-fetch');

const {resolve} = require('path');
const {existsSync, readFileSync} = require('fs-extra');
const {execSync} = require('child_process');
const {existsSync, readFileSync} = require('fs-extra');

const BASE_BRANCH = 'master';
const repo = 'polaris-react';
const sha = process.env.CIRCLE_SHA1; // eslint-disable-line no-process-env
const sha = process.env.CIRCLE_SHA1;

const postWebpackReportURL = `https://shrink-ray.shopifycloud.com/repos/${repo}/commits/${sha}/reports`;

Expand All @@ -25,10 +25,10 @@ if (sha) {
console.log(
'sha is not available, building bundle without pinging shrink-ray',
);
buildPackages(false);
buildPackages();
}

function buildPackages(includeReport) {
function buildPackages() {
execSync('yarn run webpack --config shrink-ray-build/webpack.config.js', {
stdio: 'inherit',
});
Expand All @@ -41,7 +41,7 @@ function setupShrinkRay() {
console.log(`[shrink-ray] status: ${response.status}`);
console.log(`[shrink-ray] statusText: ${response.statusText}`);
console.log('[shrink-ray] shrink-ray prebuild script completed.');
buildPackages(true);
buildPackages();
return postReportToShrinkRay();
})
.then((response) => {
Expand Down
7 changes: 2 additions & 5 deletions scripts/build.js
@@ -1,15 +1,15 @@
/* eslint-disable no-console */

import {execSync} from 'child_process';
import {ensureDirSync, writeFileSync, readFileSync} from 'fs-extra';
import {join, resolve as resolvePath} from 'path';
import {ensureDirSync, writeFileSync, readFileSync} from 'fs-extra';
import {rollup} from 'rollup';
import {cp, mv, rm} from 'shelljs';
import copyfiles from 'copyfiles';

import createRollupConfig from '../config/rollup';
import generateSassBuild from './sass-build';
import packageJSON from '../package.json';
import generateSassBuild from './sass-build';

const root = resolvePath(__dirname, '..');
const build = resolvePath(root, 'build');
Expand Down Expand Up @@ -38,9 +38,6 @@ rm('-rf', resolvePath(root, 'types/src'));

mv(resolvePath(intermediateBuild, 'src/*'), intermediateBuild);

const srcReadme = resolvePath(root, './src/components/README.md');
Copy link
Member Author

Choose a reason for hiding this comment

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

These variables were never used

const destinationReadme = resolvePath(docs, './components/README.md');

copy(['./src/**/*.md', docs], {up: 1}).catch((error) => {
console.error(error);
process.exit(1);
Expand Down
2 changes: 1 addition & 1 deletion scripts/deploy.js
Expand Up @@ -5,7 +5,7 @@ const {resolve} = require('path');
const Uploader = require('@shopify/js-uploader');
const {S3} = require('aws-sdk');
const semver = require('semver');
const awsConfig = require('../secrets.json').aws; // eslint-disable-line import/no-unresolved
const awsConfig = require('../secrets.json').aws;
const currentVersion = require('../package.json').version;

// Check if the current version is stable
Expand Down
2 changes: 1 addition & 1 deletion scripts/optimize.js
@@ -1,9 +1,9 @@
/* eslint-disable no-console */

const {resolve: resolvePath, basename, dirname} = require('path');
const SVGO = require('svgo');
const glob = require('glob');
const {paramCase} = require('change-case');
const {resolve: resolvePath, basename, dirname} = require('path');
const {readFileSync, writeFileSync, removeSync} = require('fs-extra');

const {svgOptions} = require('@shopify/images/optimize');
Expand Down
12 changes: 6 additions & 6 deletions scripts/pa11y-utilities.js
Expand Up @@ -26,12 +26,12 @@ function shitlistCheck(results, immutableShitlist) {
});

Object.keys(mutableShitlist).forEach((key) => {
mutableShitlist[key].length
? remainingIssues.push({
pageUrl: key,
issues: mutableShitlist[key],
})
: undefined;
if (mutableShitlist[key].length) {
remainingIssues.push({
pageUrl: key,
issues: mutableShitlist[key],
});
}
});

return {
Expand Down
3 changes: 1 addition & 2 deletions scripts/pa11y.js
@@ -1,7 +1,6 @@
/* eslint-disable no-console */
const puppeteer = require('puppeteer');
const pa11y = require('pa11y');
const fs = require('fs');
const shitlistCheck = require('./pa11y-utilities.js').shitlistCheck;

const shitlist = require('./../a11y_shitlist.json');
Expand Down Expand Up @@ -62,7 +61,7 @@ async function runPa11y() {
),
);

urls.map((path) => {
urls.forEach((path) => {
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 stops eslint complaining that there is no return value as in this case we don't care about the return value

const currentBrowser = browsers[browserIndex % NUMBER_OF_BROWSERS];
browserIndex++;
currentBrowser.taken = currentBrowser.taken.then(async () => {
Expand Down
2 changes: 1 addition & 1 deletion scripts/readme-update-version.js
@@ -1,8 +1,8 @@
/* eslint-disable no-console */

const {resolve} = require('path');
const {execSync} = require('child_process');
const {writeFileSync, readFileSync} = require('fs-extra');
const {resolve} = require('path');
const {version: newVersion} = require('../package.json');
const {semverRegExp, readmes} = require('./utilities');

Expand Down
2 changes: 1 addition & 1 deletion scripts/sass-build.js
@@ -1,5 +1,6 @@
/* eslint-disable no-console */

import {basename, resolve, join, relative} from 'path';
import glob from 'glob';
import {
writeFileSync,
Expand All @@ -10,7 +11,6 @@ import {
lstatSync,
existsSync,
} from 'fs-extra';
import {basename, resolve, join, relative} from 'path';
import {cp, mkdir} from 'shelljs';
import archiver from 'archiver';

Expand Down
18 changes: 2 additions & 16 deletions sewing-kit.config.ts
@@ -1,6 +1,5 @@
import {ConfigurationCallback, Env, Plugins} from '@shopify/sewing-kit';
import {join} from 'path';
import {pathsToModuleNameMapper} from 'ts-jest/utils';
import {ConfigurationCallback, Env, Plugins} from '@shopify/sewing-kit';

const tests = join(__dirname, 'tests');

Expand All @@ -10,14 +9,10 @@ export default function sewingKitConfig(
): ReturnType<ConfigurationCallback> {
return {
name: 'polaris',
library: true,
plugins: [
plugins.jest((config) => {
config.roots = [join(__dirname, 'src'), join(__dirname, 'tests')];
config.modulePaths = [
'<rootDir>/node_modules/',
'<rootDir>/src/',
'<rootDir>/tests/',
];

config.setupFiles.push(join(tests, 'setup.ts'));

Expand Down Expand Up @@ -53,15 +48,6 @@ export default function sewingKitConfig(
'!src/**/*.test.{ts,tsx}',
];

// Can be removed once SK is updated to latest ts-jest
config.transform = {
...config.transform,
'\\.tsx?$': 'ts-jest',
};

// Can be removed once SK is updated to latest ts-jest
delete config.globals['ts-jest'].useBabelrc;

return config;
}),
],
Expand Down
4 changes: 2 additions & 2 deletions src/components/AccountConnection/AccountConnection.tsx
@@ -1,6 +1,6 @@
import * as React from 'react';
import {Avatar, buttonFrom, Card, Stack, TextStyle} from '../../components';
import SettingAction from '../../components/SettingAction';
import {Avatar, buttonFrom, Card, Stack, TextStyle} from '..';
import SettingAction from '../SettingAction';
import {Action} from '../../types';
import * as styles from './AccountConnection.scss';

Expand Down
2 changes: 1 addition & 1 deletion src/components/AppProvider/AppProvider.tsx
Expand Up @@ -4,13 +4,13 @@ import {ClientApplication} from '@shopify/app-bridge';

import {LinkLikeComponent} from '../UnstyledLink';

import ThemeProvider, {Theme} from '../ThemeProvider';
import Intl from './Intl';
import Link from './Link';
import StickyManager from './StickyManager';
import ScrollLockManager from './ScrollLockManager';
import {createAppProviderContext} from './utils';
import {polarisAppProviderContextTypes, TranslationDictionary} from './types';
import ThemeProvider, {Theme} from '../ThemeProvider';

export interface Props {
/** A locale object or array of locale objects that overrides default translations */
Expand Down
10 changes: 5 additions & 5 deletions src/components/AppProvider/types.ts
@@ -1,16 +1,16 @@
import * as PropTypes from 'prop-types';
import {ValidationMap} from 'react';
import {ClientApplication} from '@shopify/app-bridge';
import {Props as AppProviderProps} from '../AppProvider';

import Intl from './Intl';
import Link from './Link';
import StickyManager from './StickyManager';
import ScrollLockManager from './ScrollLockManager';
import {
THEME_CONTEXT_TYPES as polarisTheme,
ThemeContext,
} from '../ThemeProvider';
import Intl from './Intl';
import Link from './Link';
import StickyManager from './StickyManager';
import ScrollLockManager from './ScrollLockManager';
import {Props as AppProviderProps} from '.';

export const polarisAppProviderContextTypes: ValidationMap<any> = {
polaris: PropTypes.any,
Expand Down
2 changes: 1 addition & 1 deletion src/components/AppProvider/utils/index.tsx
Expand Up @@ -67,7 +67,6 @@ export function withAppProvider<OwnProps>() {
| React.ComponentClass<OwnProps & WithAppProviderProps> & C
| React.SFC<OwnProps & WithAppProviderProps> & C,
): React.ComponentClass<OwnProps> & C {
// eslint-disable-next-line react/prefer-stateless-function
class WithProvider extends React.Component<OwnProps, never> {
static contextTypes = WrappedComponent.contextTypes
? merge(WrappedComponent.contextTypes, polarisAppProviderContextTypes)
Expand Down Expand Up @@ -139,6 +138,7 @@ export function withSticky() {
| React.ComponentClass<OwnProps & WithAppProviderProps> & C
| React.SFC<OwnProps & WithAppProviderProps> & C,
): any & C {
// eslint-disable-next-line shopify/react-initialize-state
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure what the correct fix is here so I hand waved it away as this is deep dark typescript magic that I didn't want to mess with

Copy link
Member

Choose a reason for hiding this comment

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

I think there is an issue with the order of the generics here. <{}, OwnProps & WithAppProviderProps> would mean {} is the props and OwnProps & WithAppProviderProps is the state which is probably why you're getting this error. Try changing it to <OwnProps & WithAppProviderProps, never>.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm scared, and @AndrewMusgrave was poking around this area in https://github.com/Shopify/polaris-react-deprecated/pull/2274.

I think that PR might solve this, and then because we've got the eslint-comments plugin it'll remind us to delete the ignore line if it is not needed

class WithStickyManager extends React.Component<
{},
OwnProps & WithAppProviderProps
Expand Down
4 changes: 2 additions & 2 deletions src/components/Autocomplete/Autocomplete.tsx
@@ -1,11 +1,11 @@
import * as React from 'react';

import {withAppProvider, WithAppProviderProps} from '../AppProvider';
import {ComboBox} from './components';
import {PreferredPosition} from '../PositionedOverlay';
import {OptionDescriptor} from '../OptionList';
import {ActionListItemDescriptor} from '../../types';
import {TextFieldProps, Spinner} from '../../components';
import {TextFieldProps, Spinner} from '..';
import {ComboBox} from './components';

import * as styles from './Autocomplete.scss';

Expand Down
Expand Up @@ -11,8 +11,8 @@ import Popover from '../../../Popover';
import {PreferredPosition} from '../../../PositionedOverlay';
import {ActionListItemDescriptor, Key} from '../../../../types';
import {contextTypes} from '../types';
import {TextField} from './components';
import KeypressListener from '../../../KeypressListener';
import {TextField} from './components';

const getUniqueId = createUniqueIDFactory('ComboBox');

Expand Down
@@ -1,10 +1,7 @@
import * as React from 'react';
import {autobind} from '@shopify/javascript-utilities/decorators';

import {
TextField as BaseTextField,
TextFieldProps,
} from '../../../../../../components';
import {TextField as BaseTextField, TextFieldProps} from '../../../../..';
import {contextTypes} from '../../../types';

export default class TextField extends React.PureComponent<
Expand Down
2 changes: 2 additions & 0 deletions src/components/Avatar/Avatar.tsx
Expand Up @@ -11,6 +11,8 @@ export type Size = 'small' | 'medium' | 'large';

const STYLE_CLASSES = ['one', 'two', 'three', 'four', 'five', 'six'];
const AVATAR_IMAGES = Object.keys(avatars).map(
// import/namespace does not allow computed values by default
// eslint-disable-next-line import/namespace
(key: keyof typeof avatars) => avatars[key],
);

Expand Down