Skip to content

Commit

Permalink
fix(eslint): statSync also breaks if packages does not exist
Browse files Browse the repository at this point in the history
  • Loading branch information
JounQin committed Sep 13, 2019
1 parent b15f1cf commit 0f7a37f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
9 changes: 6 additions & 3 deletions packages/eslint-config/_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ exports.isMonorepo =
fs.existsSync(path.resolve('lerna.json')) ||
!!require(path.resolve('package.json')).workspaces

const pkgs = path.resolve('packages')

exports.allowModules = exports.isMonorepo
? fs.statSync('packages').isDirectory() &&
? fs.existsSync(pkgs) &&
fs.statSync(pkgs).isDirectory() &&
fs
.readdirSync('packages')
.map(pkg => require(path.resolve(`packages/${pkg}/package.json`)).name)
.readdirSync(pkgs)
.map(pkg => require(path.resolve(pkgs, pkg, 'package.json')).name)
: undefined
3 changes: 3 additions & 0 deletions packages/eslint-config/overrides.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ exports.mdx = Object.assign({}, exports.react, {
exports.jest = {
files: '*.{spec,test}.{js,jsx,ts,tsx}',
extends: ['plugin:jest/recommended'],
rules: {
'node/no-extraneous-import': 0,
},
}

let tslint = false
Expand Down

4 comments on commit 0f7a37f

@tunnckoCore
Copy link

@tunnckoCore tunnckoCore commented on 0f7a37f Sep 13, 2019

Choose a reason for hiding this comment

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

Hey, I'm watching this repo, because it's amazing and your work at all.

Regarding this "workspaces" resolving and getting a list of packages in workspaces, I had created @tunnckocore/utils package before some weeks, that is getting all the names in all possible/defined workspaces (not only the packages/ dir), and a createAliases useful for getting a map of key/value pairs, useful for "alias" or TS's "paths", Jest or other resolvers. You can check the tests there too.

It's working for pure Lerna, or Lerna with Yarn, or Pnpm.

https://github.com/tunnckocorehq/hela/blob/v3-next/%40tunnckocore/utils/src/index.js#L50

@JounQin
Copy link
Member Author

@JounQin JounQin commented on 0f7a37f Sep 13, 2019

Choose a reason for hiding this comment

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

Thanks for sharing, it is very first version for my workspace solution, and should meet most cases(of course it could be better by your solution, thx), and TS's paths can be automatically handlered by eslint-import-resolver-ts.

I have never used Pnpm before, I will try sometime, maybe.

And thanks for sharing your repository again, I will review it deeper later.

@tunnckoCore
Copy link

@tunnckoCore tunnckoCore commented on 0f7a37f Sep 13, 2019

Choose a reason for hiding this comment

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

and TS's paths can be automatically handlered by eslint-import-resolver-ts.

What that means, more specifically? I need to have it working for all the defined workspaces, is it that you were talking about?

Like here https://github.com/tunnckocorehq/hela/blob/v3-next/tsconfig.json#L15-L18, which also is inside all of the package's tsconfig.gentypes.json. Btw, if you wonder why I don't use extends, it's because it wasn't working well for some reason, so I decided to duplicate everything 😆

@JounQin
Copy link
Member Author

Choose a reason for hiding this comment

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

What that means, more specifically? I need to have it working for all the defined workspaces, is it that you were talking about?

I mean if you are using eslint-plugin-import, then the resolver plugins should be able to resolve ts paths correctly very well.

Btw, if you wonder why I don't use extends, it's because it wasn't working well for some reason, so I decided to duplicate everything

Personally, when I use ts project reference feature, I have to do this also.

Please sign in to comment.