search

rescript-lang / rescript-compiler

The compiler for ReScript.
https://rescript-lang.org
Other
6.68k stars 445 forks source link

Pinned packages support #4361

Closed BlueHotDog closed 3 years ago

BlueHotDog commented 4 years ago

https://github.com/BlueHotDog/reason_monorepo

Monorepo support via common tooling(specifically in the above repo Lerna) is broken. Monorepo support for JS projects is usually being done by using symlinks to support cross-project dependencies - That's the most basic and common workflow. Another possible monorepo approach is by using Yarn workspaces, which utilizes symlinking even more by hoisting all dependencies to the "root".

Currently, bucklescript is unable to handle any kind of monorepo setup, which, after trying to bang my head, is impossible to solve and leads to a very weird workflow for any ambitious, project(e.g a modern SPA + Nodejs server in Reason).

The example project above illustrates all the problems, but just to summarize what i've encountered:

Links to previous discussions that died out and might be related: https://github.com/BuckleScript/bucklescript/issues/3521 https://github.com/BuckleScript/bucklescript/pull/4047

As there's no roadmap for reasonml. any type of communication regarding stale issue would really help.

jchavarri commented 4 years ago

Another issue that makes the monorepo experience challenging is #4084. An example use case that we've seen is the generation of .ml and .mli files from .atd files. Because there are BuckleScript libraries in the monorepo that depend on .atd files, and on have dependents as well, we have to run atdgen "off-bsb" with Dune, but this means that bsb doesn't know anything about how these targets are generated, so one has to run scripts for every change to the source .atd files.

TheSpyder commented 4 years ago

Other references to monorepo requests, so we can centralise the discussion

1950

2750

3528

TheSpyder commented 4 years ago

I have forked the monorepo @BlueHotDog posted and rewritten it to show the style of monorepo I would like to use. I've written up all the things this is designed to achieve, and why it doesn't currently work, in the readme: https://github.com/TheSpyder/reason_monorepo

This structure holds a lot of potential, I very nearly used it for my production project, but the watch-rebuild process is just slow enough to be annoying and the lack of compiler warnings really killed it for us.

BlueHotDog commented 4 years ago

@TheSpyder that's awesome! thank you(hopefully BS gods will hear this) One requirement that you didnt list which is needed for me is the ability to run each project separately: i'm developing a chrome extension + front-end and need to be able to run bsb+webpack + etc from two different packages side by side.

The way i'm doing it now, e.g by having a single top level bs-config makes this impossible(you have to run a single build, so when working on the flow i've to:

TheSpyder commented 4 years ago

@BlueHotDog you can still run each project separately if you want, even in my example 😁

Little-known fact about npm module resolution, it also supports searching for a parent package.json and node_modules folder. This is what makes yarn workspaces work so well.

reason_monorepo spyder$ yarn
<stuff here>
reason_monorepo spyder$ yarn build
<build output>
reason_monorepo spyder$ cd packages/a
a spyder$ bsb
[18/18] Building src/FetchedDogPictures/FetchedDogPictures-A.cmj

packages/b also works, but that's where I have the warning-as-error configured.

In fact, my setup might work for you today if you never use a top-level bsb.

harigopal commented 4 years ago

I actually work on a monorepo that uses Lerna + Yarn workspaces - Pupilfirst.

It's not a traditional workspaces-based monorepo, in that there is a top-level application, with other packages inside it; they're not all side-by-side like in the example @TheSpyder created.

And it works... most of the time.

Occasionally, the bsb compilation will start failing, complaining about the presence of duplicate .re files (files with the same name). At that point, one of the packages (app/javascript/packages/pf-icon) will have a lib/ocaml folder inside it that contains dupes of some Reason files. Deleting that folder gets the compilation going again... until seemingly at random, the lib/ocaml folder shows up again.

It's a bit annoying, but it happens rarely enough that it doesn't get in the way of work, and it was the only way I could set this up as a monorepo.

I haven't a clue why this happens - I haven't been able to replicate it reliably, but it probably has something to do with all packages (including root) having their own bsconfig.json files - and two of the packages depend on another -pf-icon... and it's always pf-icon that gets the weird lib/ocaml folder with the dupe files inside it.

TheSpyder commented 4 years ago

@harigopal your top-level bsconfig.json also loads all the packages as source, which is the technique I ended up using in production, unfortunately that means the packages can't leverage namespace: true.

My guess about your lib/ocaml problem is the language server occasionally realises there is another bsconfig.json and compiles that folder.

BlueHotDog commented 4 years ago

Seems that on BS 8 the situation is better and now i can work with Yarn Workspaces.(was this fixed? couldnt see anything in any communications nor was it on the roadmap.. but.. yeah)

TheSpyder commented 4 years ago

I'm not sure what changed, but I don't believe it has been fixed. I'm still struggling along without access to namespace:true.

If you don't want to keep this open, I'll open a new one 🤔

BlueHotDog commented 4 years ago

Yup, my mistake, wasn't fixed. @bobzhang is this still planned for the next releases as documented in the roadmap?

BlueHotDog commented 4 years ago

@bobzhang any update on this? is this planning to be getting some attention? unfortunately if this is a no-go i can't use Reasonml and need to move code back to typescript i would just love to know before i make any drastic changes.

bobzhang commented 4 years ago

Hi, sorry for your bad experience. It’s on our roadmap for this half so we will have a look . But I am not sure if we can deliver it in a timely manner to match your expectation.

Danni Friedland notifications@github.com于2020年8月10日 周一下午9:55写道：

@bobzhang https://github.com/bobzhang any update on this? is this planning to be getting some attention? unfortunately if this is a no-go i can't use Reasonml and need to move code back to typescript i would just love to know before i make any drastic changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BuckleScript/bucklescript/issues/4361#issuecomment-671368746, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMK5MDLJJJQ2FO35UJRLR7735XANCNFSM4M4ORL5A .

-- Regards -- Hongbo Zhang

BlueHotDog commented 4 years ago

Hopefully, if i could get some visibility into what's planned to ship(and a ballpark of when) i can make an educated decision with the team.

bobzhang commented 4 years ago

I will think about it and get back to u later this week

Danni Friedland notifications@github.com于2020年8月10日 周一下午10:53写道：

Hopefully, if i could get some visibility into what's planned to ship(and a ballpark of when) i can make an educated decision with the team.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BuckleScript/bucklescript/issues/4361#issuecomment-671403912, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMKYXVEBK5PFXOODG5J3SAACWFANCNFSM4M4ORL5A .

-- Regards -- Hongbo Zhang

bobzhang commented 4 years ago

Hi Danni, sorry for being late, I checked my calendar that we'll start the improvement over build system in general in mid September, this is an issue we will prioritize.

On Mon, Aug 10, 2020 at 10:53 PM Danni Friedland notifications@github.com wrote:

Hopefully, if i could get some visibility into what's planned to ship(and a ballpark of when) i can make an educated decision with the team.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BuckleScript/bucklescript/issues/4361#issuecomment-671403912, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMKYXVEBK5PFXOODG5J3SAACWFANCNFSM4M4ORL5A .

-- Regards -- Hongbo Zhang

BlueHotDog commented 4 years ago

Hey Bob, That'll work. please feel free to reach out for input/tests any assistance. happy to spend time on making this as best as we can make it.

Thanks!

TheSpyder commented 4 years ago

I'm also happy to help with thoughts on how changes will impact my attempts to develop in a monorepo

itayadler commented 4 years ago

Hey guys, any news on this issue? I've been working heavily these past few weeks on a ReScript monorepo, and I experience pains with setting up a consistent watch flow, between 2 packages (app, common), it keeps breaking whenever I change something in the common package, and then I need to run a clean build on both packages in order to get it working

bobzhang commented 4 years ago

Hi, we are working on the improvement of build system currently, will get back to you if we need more feedback

On Mon, Sep 14, 2020 at 4:05 AM Itay Adler notifications@github.com wrote:

Hey guys, any news on this issue? I've been working heavily these past few weeks on a ReScript monorepo, and I experience pains with setting up a consistent watch flow, between 2 packages (app, common), it keeps breaking whenever I change something in the common package, and then I need to run a clean build on both packages in order to get it working

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rescript-lang/rescript-compiler/issues/4361#issuecomment-691718354, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMKZU2OMQTI23OLF2KSLSFUQZHANCNFSM4M4ORL5A .

-- Regards -- Hongbo Zhang

itayadler commented 4 years ago

sweet, ty 🙇 would love to help anyway I can

BlueHotDog commented 4 years ago

Hey, i see there's a lot of progress on 8.3, and i was eager to try and see if we get good Monorepo support. Not sure if this release is intended to fix these issue, but it seems like the problems are still there - Is 8.3 intends to address better support for monorepo?

bobzhang commented 4 years ago

Hi, it is not in this release. As you can see, we are working on build system stuff in this release, we will keep working on build system related stuff in next release

On Wed, Sep 23, 2020 at 11:25 PM Danni Friedland notifications@github.com wrote:

Hey, i see there's a lot of progress on 8.3, and i was eager to try and see if we get good Monorepo support. Not sure if this release is intended to fix these issue, but it seems like the problems are still there - Is 8.3 intends to address better support for monorepo?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rescript-lang/rescript-compiler/issues/4361#issuecomment-697539010, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMK7YSPSGSMURLYQ57ODSHIHPHANCNFSM4M4ORL5A .

-- Regards -- Hongbo Zhang

itayadler commented 4 years ago

@bobzhang exciting stuff about 8.3! myself and the company I work for really love ReScript and this issue is affecting us on a daily basis, as we need to workaround it each time it occurs. If there was a Patreon for the project myself and the company would be the first to signup to help and prioritise this issue somehow.

TheSpyder commented 4 years ago

If there was a Patreon for the project

As far as I understand they're all facebook employees, I don't think they need patreon support 😂

fakenickels commented 3 years ago

Really eager to see this happening! We have a pretty big monorepo here and we suffer in a daily basis with the problems pointed above. If there is any way we can help testing you can count with us @bobzhang. Thanks for your effort

Freddy03h commented 3 years ago

I'm transitioning to a monorepo too, to share code between the react-native and the react-native-web apps essentially.

For now I launch bsb watch in each packages I needed, with the same package-specs (module: es6, in-source: true), It work while working on implementation detail (example : doing styles in packages/design-system while working on the package/react-native-app), but doesn't if I change parameters of a componentd for example.

fakenickels commented 3 years ago

That's exactly it @Freddy03h. Also, even in a setup like this the problem seems to be the same https://github.com/fhammerschmidt/bucklescript-monorepo

BlueHotDog commented 3 years ago

@bobzhang @Hongbo-bb any update on this? I see a lot of work being done on PPX, is there any progress on this issue, or is PPX a prerequisite?(Or just more important which is fine, i just would love some visibility and expectation setting)

Freddy03h commented 3 years ago

In fact, bsb create the .bsb.js files for all the bs-dependencies in the node_modules (including monorepo packages) at launch time (of course otherwise it wouldn't work). But bsb doesn't watch for changes in the dependencies like webpack or metro do (you can monkey patch a js file in node_modules and changes will be watched).

bobzhang commented 3 years ago

Hi I am starting to look at this issue, shall we have a video chat on the pain points? Note some pain points are independent which will be delayed later:

Khady commented 3 years ago

How do we get someone from Ahrefs to participate to the video chat?

TheSpyder commented 3 years ago

I'm keen to participate, but I'm in Australia which usually means chats like this are at 3am or similar.

Coobaha commented 3 years ago

Keen to participate too, Helsinki, we do have monorepo setup at Wolt

Also another pain point worth to mention but probably very hard to solve: transitive dependencies for published packages. When there is another RE(S) dependency - we can't just ship JS output of rescript-compiler... We need to rebuild all top level packages on consumer side too, because node_modules are usually shipped as is (re code)

BlueHotDog commented 3 years ago

I'm excited like a groom on his wedding day. @bobzhang - Maybe some hours and we can vote or something? seems like a lot of interested people(Nice opportunity for the reasonml community to join in)

fakenickels commented 3 years ago

I'm keen to participate as well

bobzhang commented 3 years ago

I live in +8 timezone. 9am - 10pm would work for me. What would be the best time to cover most people? Do you have experience in setting up team meetings. The zoom free versions look good to me (the limits are 100 people, 40 minutes which look fine.)

To make the meeting efficient, it would be nice that if you can share some reproducible build failures.

Note there's one issue that should be solved immediately, the current way of bsb -make-world walking directory in a non-deterministic way (depends on the OS behavior)

TheSpyder commented 3 years ago

You have a lot of cross-over with me in +10, although I would not be available after about 8pm your time.

The example use case I posted earlier in this thread is still my ideal setup: https://github.com/rescript-lang/rescript-compiler/issues/4361#issuecomment-629780898 and the example repo at https://github.com/TheSpyder/reason_monorepo

For the last 12 months I have been working around the inability to achieve this use case by adding all project source directories to the top-level bsconfig.json, flattening the monorepo into a single project. This has caused significant overheads in namespace management.

jchavarri commented 3 years ago

I noticed that both examples above (1, 2) use Lerna+Yarn, so here's another repro that just uses Yarn workspaces to show the problem with watcher: https://github.com/jchavarri/bsb-smallest-monorepo-example.

At Ahrefs, our workaround as well has been to flatten the monorepo into single bsb "project" (i.e. 1 bsconfig.json). As @TheSpyder mentioned, this solved the issues related to monorepo (watcher, generators not being transitive which broke atdgen, etc) but complexity moved to other places (single namespace leads to a lot of conflicts, tooling for auto-generation of bsconfig.json files for "per-project" local development).

I'm in EU time (CET), but have flexibility to wake up early or stay late if needed. @bobzhang I would suggest that you set a time as the organizer, and people try to accommodate. With folks in Asia, America and Europe, it'll be challenging to find a time that suits everyone :)

Also, would it be interesting to record the call for those that are not able to join? It seems Zoom allows it on free plans.

bobzhang commented 3 years ago

Hi all I made a post here https://forum.rescript-lang.org/t/rescript-office-hours-3/582

TheSpyder commented 3 years ago

I noticed that both examples above (1, 2) use Lerna+Yarn

Oh I forgot I left that in there. I don't use lerna anymore, I don't think we ever leveraged it for the ReasonML project I'm working on before it was removed.

leeor commented 3 years ago

I am using a monorepo based on yarn workspaces.

Given a monorepo with packages A, B, C, when A depends on both B & C, and B depends on C, when working on module A there are two main pains in my experience:

  1. You can either build the specific module you're in or all dependencies, but not just dependencies from the local repo. Since the watcher doesn't trigger for dependencies and I did not want to flatten the repo (namespacing), I am left with having to run a bsb -make-world - which leads to the next issue.
  2. Very often, changes to C will require a complete rebuild of A because it will be in conflict with interfaces in B.

Ideally, we will have the option to run bsb in the monorepo root and it will build (or watch) everything while maintaining dependencies across modules so that cleaning will not be required very often for small changes.

I can describe these scenarios in more detail if needed.

ixzzd commented 3 years ago

Very often, changes to C will require a complete rebuild of A because it will be in conflict with interfaces in B.

@leeor I had the same issue, but can't make small repro. Maybe you know how to reproduce it? You are talking about staled build issue, right?

bobzhang commented 3 years ago

please chime in on this thread if you have concerns on pinned packages support https://forum.rescript-lang.org/t/call-for-testing-bs-platform-8-3-3-dev-1-better-make-world-performance/616/5

BlueHotDog commented 3 years ago

Writing here to make sure issue is well documented and easy to find for newcomers: "I have read all feedback available on the mono-repo support.

Let me explain the original design and why it works in current way.

Note the name is a bit misleading, mono-repo structure is simple and well supported, here users want to support develop multiple packages in separate repos while achieve the similar experience to monorepo.

what does -make-world do

-make-world will first resolves each package recursively and build them independently, this has to be done whether there are changes to the package or not, the changes are not tracked by the build system, that’s one of the reasons why it is slow

The building is on a different mode from toplevel projects (dependency mode):

The warning/errors are mostly ignored, this is designed with the mindset that if your dependency has a warn-error, it should not get in your way. Custom rules are not run The motivation is that your dependency should not impose his build dependency to its consumers. There’s an installation process after finishing building the package. So for a package a, the user wants to hide some interface from its consumer, he can choose not to install that cmi file. This abstraction is implemented in the installation. Installation is basically copying files which can not be as fast as we hope

What we can improve on current situation? We can fix the dependency mode as long as we have user input that some packages are pinned packages that it should go to toplevel mode instead of dependency mode.

We could bypass the installation for pinned packages. That means for pinned packages, every interface is exposed (minor semantics difference from unpinned packages)

Watcher

Our built-in watcher is not great due to the limitation of cross platform and minimal dependencies. We want to offer the help to make it easier to integrate 3rd party watchers e.g, watchman

Merge multiple pinned packages into a monorepo?

In theory, the build system could scan multiple pinned packages and treat it as a monorepo, this is a lot of work. However much work we have done here, the semantics would not be the same as multiple packages since the generated js files are path sensitive, even if we make sure all relative paths are still calculated correctly, it is hard to preserve the semantics with bindings, The benefit is of course performance.

Suggestions

If you truly own these packages, I would encourage you adopt a real monorepo setup, then you don’t have these problems at all. For pinned packages, we will discuss how we can improve here.

Bugs

There are some other minor bugs about path resolving which should be fixed later."

BlueHotDog commented 3 years ago

Bug fixes when dependencies changed There are several obvious bugs when you change your dependencies but will be fixed, hope this alone would make your life easier:

Installation

the installation was not always triggered for some reasons, This was correct if you don’t change the depency, but seems more people will change the deps, we are going to fix this. Scan the directory in a deterministic way && Lock the deps in the bs-depenencies Previously, the scanning order depends on the OS specific behavior, and re-do a make-world, will rescan, we will fix this non-deterministic behavior, and lock the bs-dependencies once it’s scanned, this probably contributes to your inconsistent assumption error Toplevel mode and dependency mode The two modes are slightly different, for example, dependency mode does not run generators, it makes sense. However, we will apply toplevel mode to pinned packages too so that it generators are always re-run when the source changes

Watcher issues

We plan to make the build system 3rd party friendly so that you can query which directories to watch and integrate with your favorite watcher.

Other issues

Concurrent build Suppose both A and B depends on C. -make-world A and -make-world B could potentially trigger a concurrent build on C. We are not sure to allow concurrent build in the same directory, but at least we will lock the build directory and spits out a meaningful error. Other high level changes to the build system

Proposed UI Changes: subcommands mode

The current command line flags are a bit messy and some flags are in conflict for different state. We plan to follow other tools, e.g:

npx bsb clean npx bsb build npx bsb format npx bsb install Build system bugs are hard to reproduce, if you can contribute some reproducible failure examples, that would be much appreciated! – Hongbo

BlueHotDog commented 3 years ago

Is this fixed? what's the status? i see 8.3.3 released without any details. Some sporadic discussion on the forum, but i can't figure out if this is fixed or what's the status.

TheSpyder commented 3 years ago

Looking at the git history, the 8.3.3 release was cut at least a couple of days ago, two recent PRs mentioning pinned packages were merged today.

Hongbo-bb commented 3 years ago

8.3.3 was announced on the forum with a detailed list of changes. This issue is a work in progress

On Wed, Nov 18, 2020 at 2:25 AM Danni Friedland notifications@github.com wrote:

Is this fixed? what's the status? i see 8.3.3 released without any details. Some sporadic discussion on the forum, but i can't figure out if this is fixed or what's the status.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rescript-lang/rescript-compiler/issues/4361#issuecomment-729115959, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLRMSXNMXUOXZMKNBNDKZ3SQK5Y5ANCNFSM4M4ORL5A .

-- -- Regards, Hongbo

bobzhang commented 3 years ago

Hi, we made a release of 8.4.0-dev.3 which should have most issues mentioned here solved.

Besides some bug fixes, two major fixes here:

First, dependency changes will trigger the rebuild of its descendants.

Second, we introduced a concept of pinned-dependencies, an example usage is here so that packages are classified as three categories:

Caveats:

Tests and bug fixes are welcome, we will mark this issue solved if there is no major drawbacks in the current design

fakenickels commented 3 years ago

Thanks for the update @bobzhang! I'll be trying it out with ReForm for the next few days.

I just started testing it out here: https://github.com/Astrocoders/reform/pull/192

First things I noticed: