Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

Set up import parsers so import/* rules work in typescript #197

Merged
merged 1 commit into from
Oct 29, 2018

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Oct 27, 2018

It turns out you need some extra config in order for some of the import
checking rules to do their thing with TypeScript.

To tophat:

  • Check out a project that already uses eslint-plugin-shopify
  • yarn add --dev eslint-import-resolver-typescript
  • Add the settings to your eslint config.
  • Run eslint. Marvel at all those new errors.

It turns out you need some extra config in order for some of the import
checking rules to do their thing with TypeScript.
Copy link
Member

@lemonmade lemonmade left a comment

Choose a reason for hiding this comment

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

This is great, good find. I think we still need some more configuration for our default project setups (resolving packages/* and so forth), but this is a great start.

@BPScott
Copy link
Member Author

BPScott commented Oct 29, 2018

I think we still need some more configuration for our default project setups (resolving packages/* and so forth)

Adding "typescript": {}, shall automatically look into the top-level tsconfig.json and read the paths and baseUrl from there so I think this works out the box.

I just tried adding import Asset from '@shopify/sewing-kit-server' into /packages/sewing-kit-server/assets in web and it gives me a "dependency cycle detected" warning.

@lemonmade
Copy link
Member

Oh, awesome. A great start and finish then :P

Copy link
Contributor

@patsissons patsissons left a comment

Choose a reason for hiding this comment

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

nice work!

@patsissons
Copy link
Contributor

will this be a point release? should i try and roll it into https://github.com/Shopify/sewing-kit/pull/1038 ?

@BPScott
Copy link
Member Author

BPScott commented Oct 29, 2018

This feels like a minor release (26.1.0) - technically it's fixing a config that /should/ have always been working, but it's going to introduce a lot of new linting messages.

Rolling it into the new SK release sounds like a good plan.

@BPScott BPScott merged commit 3eec4ad into master Oct 29, 2018
@BPScott BPScott deleted the typescript-imports branch October 29, 2018 17:39
@BPScott
Copy link
Member Author

BPScott commented Oct 29, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants