Fix open compiled command for monorepos

[fhammerschmidt]

When you for instance set the suffix in your root bsconfig in a monorepo, but omit it in the nearest bsconfig to your ReScript file, the open compiled command won't work. This PR addresses that. I tried to fix it gracefully, i.e. only when no file is found with the current approach, it tries to go one directory up and find the root bsconfig from there. Then the already existing bsconfig-processing logic is applied on the root bsconfig to find the correct suffix/module system.

[fhammerschmidt]

@zth Would be nice to get this in soon.

[zth]

1. Can we get rid of the any here? Dunno if it's introduced in this PR or if you're just explicitly stating something that was implicit before, but it would be nice to see if we can get rid off them.
2. Has this been thoroughly tested both on monorepo setups and non-monorepo setups?

[fhammerschmidt]

Ad 1.: Added build-schema types (converted from the compilers original JSON schema)
Ad 2.: Used it for the last two weeks on my projects, @cknitt tested it as well. I also worked on a real-world non-monorepo and the command still worked.

[zth]

All good from me! Ok with you as well @cristianoc ?

[cristianoc]

Left some comments about consolidating the search logic more.
I guess there's more analogous logic for the binary, and putting both in the same place could be useful to keep them in sync.

[cristianoc]

This logic is a little strange, but I guess monorepos are unspecified at the moment, so looking blindly at the parent dir is one way of doing it.

Of course, this might not be a monorepo, or it could be that the file simply is not compiled, and we start searching in random places.

I guess these are all current limitations.
This logic, and any such logic, should probably live together in a dedicated file, so it's easier to update in the future when things become less unspecified.

[fhammerschmidt]

What exactly should be moved into a separate file? The recursive functions findProjectRootOfFile and findFilePathFromProjectRoot or the new logic I introduced with this PR (what is inside the !fs.existsSync if-block)?

[cristianoc]

I would say just the logic.
I.e. go looking in the parent, find the root from there, take the info from the root bsconfig.

I guess the logic for the binary is similar and if they go to the same place it's easy to keep them in sync.

[fhammerschmidt]

This will introduce circular dependencies. I know this will work in TypeScript, but I think it is not good regardless.
Except I move the aforementioned lookup methods to the new file as well.

[cristianoc]

Why would move some code to a brand new file introduce circular dependencies?

Unless this refers to something else.z

[cristianoc]

And this is definitely not a generic util.

[fhammerschmidt]

Assuming I keep the getCompiledFilePath in utils.ts, but move the new parts into another file. In that file I would have to access utils.findFilePathFromProjectRoot among others --> circular dep

[cristianoc]

It could be a lookup file, with a section for monorepo.

Or a lookup file and a monorepo file.

It does not really matter much as long as relevant coupled code is all in one place.

[fhammerschmidt]

Gave it a try in 6830b29

hope this is how you meant it

[cristianoc]

Looks great. Ready to merge I think.

[cristianoc]

For my understanding, could we draw some boundaries around how things can possibly be structured?

I thing the most general multi-project scenario would look like this:

• There are multiple projects (corresponding to multiple `bsconfig.json files in different dirs)
• There are multiple node_modules dirs (perhaps of different number and perhaps on completely different dirs than those where bsconfig.json files go)
• There are multiple compilation artifacts dirs (perhaps of different number and perhaps on completely different dirs than those where bsconfig.json files go).

And I'm intentionally keeping it unreasonably vague in hope one can narrow down some of these boundaries.

[fhammerschmidt]

Added a small fix (this got lost when refactoring) and tested it again agains some of our repos (including non-monorepos), all good.