Shopify / tslint-config-shopify

Shopify’s TypeScript rules and configs.
https://www.npmjs.com/package/tslint-config-shopify
37 stars 3 forks source link

Allow `import {foo}...` (no whitespace around `foo`) #74

Closed kaelig closed 7 years ago

kaelig commented 7 years ago

Fixes #73, where a regression would enforce import { foo }... instead of import {foo}....

Unfortunately there is no way to enforce import {foo} from 'foo'; with the current version of TSLint. Prettier can do it for us, though.

This also fixes a regression where this would be valid: function(){ return 'foo'; } (missing whitespace between ) and {.

GoodForOneFare commented 7 years ago

Can you describe the behaviour that this is fixing, please? I've checked out polaris-styleguide, and tslint isn't flagging any import expressions, and the autofixer isn't reformatting them.

kaelig commented 7 years ago

Did you test by removing lines 8-19 of https://github.com/Shopify/polaris-styleguide/blob/master/config/tslint/rules.js? Those are supposed to override the behavior inherited from tslint-config-shopify (unless I've dreamt it somehow…).

GoodForOneFare commented 7 years ago

Nope. Generally, including instructions like that, and providing output/screencaps of the problem will help to assess the issue, though.

Here's what I saw once I followed those instructions:

✗ yarn run lint:ts
yarn run v1.0.1
$ tslint -c ./config/tslint/full.json './{app,server,client,packages,config}/**/*.{ts,tsx}' --exclude './**/node_modules/**/*.{ts,tsx}' --project tsconfig.json --type-check --format codeFrame
/Users/gord/shopify/polaris-styleguide/app/components/AssetSVG/index.tsx
missing whitespace (whitespace)
  1 | import * as React from 'react';
> 2 | import { classNames} from '@shopify/react-utilities/styles';
    |                   ^
  3 |
  4 | import * as styles from './AssetSVG.scss';
  5 |

/Users/gord/shopify/polaris-styleguide/app/components/Backdrop/index.tsx
missing whitespace (whitespace)
  1 | import React from 'react';
> 2 | import {autobind} from '@shopify/javascript-utilities/decorators';
    |                ^
  3 | import ScrollLock from '../ScrollLock';
  4 |
  5 | const ATTR_BACKDROP = 'data-backdrop';

That's bad :/

GoodForOneFare commented 7 years ago

@ismail-syed is looking into using eslint rules to handle TypeScript formatting. Until then, this PR is much appreciated 👍

kaelig commented 7 years ago

Thanks for merging, and sorry for the lack of instructions.

I was thinking we may want to get prettier to handle styling and ESLint / TSLint to handle the rest. To be discussed!