Sewing-kit bump #2369
Sewing-kit bump #2369
Conversation
| "error", | ||
| { | ||
| "allow": [ | ||
| "UNSTABLE_Color", |
There was a problem hiding this comment.
This is a bit nicer than sprinkling single-line ignores throughout the codebase
| { | ||
| "files": ["**/*.test.{ts,tsx}"], | ||
| "rules": { | ||
| "jest/no-truthy-falsy": "off", | ||
| "shopify/jsx-no-hardcoded-content": "off", |
There was a problem hiding this comment.
This is now turned off in tests by default
| }, | ||
| "overrides": [ | ||
| { | ||
| "files": ["src/**/*.{ts,tsx}"], |
There was a problem hiding this comment.
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.
| @@ -16,7 +16,7 @@ describe('<AppProvider />', () => { | |||
| }); | |||
|
|
|||
| it('updates context when props change', () => { | |||
| const Child: React.SFC<{}> = () => { | |||
There was a problem hiding this comment.
A lot of the changes in this PR are "that's the default value for the generic, you don't need to specify that"
| @@ -5,9 +5,6 @@ import {useAppBridge} from '../../utilities/app-bridge'; | |||
|
|
|||
| export interface LoadingProps {} | |||
|
|
|||
| // 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 | |||
There was a problem hiding this comment.
We've updated the version of eslint-plugin-react to a version that fixes this false positive
|
|
||
| // Wrapping forwardRef in a memo means the component does not get a name set | ||
| // So we have to do it ourselves | ||
| // https://github.com/facebook/react/issues/16722 |
There was a problem hiding this comment.
Bug whack-a-mole. I'm not convinced we should be wrapping this in memo in the first place: #2330
| @@ -11,6 +11,7 @@ export function get<T>( | |||
|
|
|||
| const keys = Array.isArray(keypath) ? keypath : getKeypath(keypath); | |||
| let acc = obj; | |||
| // eslint-disable-next-line @typescript-eslint/prefer-for-of | |||
There was a problem hiding this comment.
fixing this causes some other error so gonna ignore it
This updates our eslint version to 6 and our eslint-plugin-shopify version to v31, which includes support for the new @typescript-eslint parser. Enable just the basic typescript rules for now
The prettier plugin is going to remove eslint integration soon, let's get ahead of the curve while sidestepping the problem that prettier-eslint doesn't work with eslint 6
| @@ -202,7 +202,7 @@ const App = () => { | |||
| <Color dim> | |||
| <Box width={3}>💡</Box> | |||
| <Box> | |||
| Tip: to disable these reports, run{' '} | |||
There was a problem hiding this comment.
This causes a regression where a space is missing between run and DISABLE_SPLASH:
| @@ -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 🤖 | |||
There was a problem hiding this comment.
No regression here – all good.
| @@ -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 | |||
WHY are these changes introduced?
This updates our eslint version to 6 and our eslint-plugin-shopify
version to v31, which includes support for the new @typescript-eslint
parser so we can get rid of that resolution hack.
WHAT is this pull request doing?
Updates sewing-kit and updates code to adhere to updated rules.
This also changes our vscode config so that the prettier extension no longer formats our
{js,ts}x?files on save and instead this is handled by the eslint extension, this is because the prettier extension doesn't play nice with eslint 6 (as it uses prettier-eslint under the hood which doesn't like eslint6: prettier/prettier-eslint#236) and its future roadmap will remove support for running eslint formatting (prettier/prettier-vscode#958).How to 🎩
linting and tests pass
After doing a
yarn installrestart VSCode and ensure that format on save still works for various file types: