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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue setting up monorepo #124

Open
robertherber opened this issue Jul 26, 2018 · 7 comments
Open

Issue setting up monorepo #124

robertherber opened this issue Jul 26, 2018 · 7 comments

Comments

@robertherber
Copy link

I'm using flowtype-errors in a monorepo setup, with this folder structure:

  • MyProject
    • moduleA
      • .eslintrc
      • .flowconfig
    • moduleB
      • .eslintrc
      • .flowconfig
    • etc..

It's working great from the command line and everything else in ESLint works great in Atom as well. But for some reason flowtype-errors gets confused regarding where the flow setup is.

Without configuration I get this error:
Flow returned an error: Could not find a .flowconfig in /Users/robert/Code/MyProject or any of its parent directories. See "flow init --help" for more info (code: 12) (flowtype-errors/show-errors)
Which indicates that it's looking for a .flowconfig in the root of the monorepo.

If I add this configuration to my .eslintrc settings:
'flowtype-errors': { 'flowDir': './moduleA' },
I get this error instead, which still feels like it's confused about the directory structure:
Invalid position given by 'flowtype-errors/show-errors'. See the description for details. Click the URL to open a new issue!

I'm not sure whether it's caused by this plugin or the Atom linter. Any ideas? :)

@jdmota
Copy link
Collaborator

jdmota commented Jul 26, 2018

I guess we should use something like find-up to find the correct .flowconfig instead of assuming that there is only one config file, on the root or in flowDir...

function lookupFlowDir(context: EslintContext): string {
const root = process.cwd();
const flowDirSetting: string =
(context.settings &&
context.settings['flowtype-errors'] &&
context.settings['flowtype-errors'].flowDir) ||
'.';
return fs.existsSync(path.join(root, flowDirSetting, '.flowconfig'))
? path.join(root, flowDirSetting)
: root;
}

Or instead, just let flow do its thing, instead of reading the .flowconfig ourselfs...

const source = context.getSourceCode();
const flowDir = lookupFlowDir(context);
// Check to see if we should run on every file
if (runOnAllFiles === undefined) {
try {
runOnAllFiles = fs
.readFileSync(path.join(flowDir, '.flowconfig'))
.toString()
.includes('all=true');
} catch (err) {
runOnAllFiles = false;
}
}
if (runOnAllFiles === false && !hasFlowPragma(source)) {
return;
}

The second error you mentioned (Invalid position...), I believe is a separate issue, and it seems to come from eslint. What version of eslint are you using? And can you provide a small reproducible example? Thanks

@robertherber
Copy link
Author

robertherber commented Jul 27, 2018

I'm using ESLint@5.2.0.

I've reproduced the error with a minimal setup here.

Open the root git folder in Atom with default linting settings and you should be able to reproduce it as well. I've only tried with Atom - but it wouldn't surprise me if it's the same with other editors.

@jdmota
Copy link
Collaborator

jdmota commented Jul 27, 2018

When you get the error Invalid position given by 'flowtype-errors/show-errors'. See the description for details. Click the URL to open a new issue!. What is the description that is given?

The problem probably is that something changed with eslint@5 and we don't support it yet.

Edit: I can't reproduce it on the command line...

@robertherber
Copy link
Author

To reproduce from the command line:
moduleA/node_modules/.bin/eslint .

Which gives the error:
/Users/robert/Code/_playground/repro-eslint-flowtype-error/moduleA/some-code.js 1:1 error Flow returned an error: Could not find a .flowconfig in /Users/robert/Code/_playground/repro-eslint-flowtype-error or any of its parent directories. See "flow init --help" for more info (code: 12) flowtype-errors/show-errors

I also verified that I get the same error with eslint 4.19.1 - so it's not an eslint 5 issue.

I don't think the "invalid position" error matters. It's probably unrelated - removing my flow-typed setup removed the error which had this description:
screenshot_2018-07-29 12 54 28_xcoeua

@robertherber
Copy link
Author

robertherber commented Jul 29, 2018

I'm thinking out loud now. Eslint itself supports recursive .eslintrc setups. Which is why it works great in monorepos. Flow does not however support it for .flowconfig. If it did it would be simple - one single flow process would be able to handle an entire monorepo (with a quick look I guess that's how this plugin works).

Would it be possible to spawn one flow process per .flowconfig instead? That's how for example Flow IDE (an Atom plugin) seems to deal with it.

For example: start by looking for a .flowconfig in the root directory. If it's not found look one or two levels recursively down and start one flow process per .flowconfig.

@jdmota
Copy link
Collaborator

jdmota commented Jul 29, 2018

For example: start by looking for a .flowconfig in the root directory. If it's not found look one or two levels recursively down and start one flow process per .flowconfig.

I think the opposite would work better: starting on the folder a file is, work up-wards with something like find-up. When we find .flowconfig, that is the root.

If we work down-wards, we would need to walk every folder or have information about your monorepo folder structure, and that means more configuration to setup...

@amilajack Any ideias?

@robertherber
Copy link
Author

Looking upwards from the actual file obviously sounds like a better idea @jdmota :)

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

No branches or pull requests

2 participants