Hi all,
Follow up with the discussions about better mono-repo support, I made a dev release(for Linux platform) which should give you a performance boost for bsb -make-world. (Note previously, we assume dependencies are just built once, so such experience are not optimized).
As always, tests on build system are appreciated, you can try it npm i bs-platform@8.3.3-dev.1.
The major changes is that the installation is tracked in the build graph so it is more correct and performant, previously we always have to re-do all the work when building its dependencies, which is now not needed.
We will keep iterating on improving the experience for multiple repos, Thanks ā Hongbo
There are no breaking changes yet.
With these changes mentioned above, you should see a noticeable speed up in -make-world nop build and hopefully no regressions
Just tried it in one of our or projects, here are my findings:
yarn install took an eternity for some reason.
āØ Done in 279.54s.
On the first run of yarn bsb -clean-world -make-world, I got the following errors:
sb: error: build.ninja:15: expected '=', got identifier
build src/Persistence.reast : build_ast_from_re $src_root_dir/src/Persi...
^ near here
Failed
bsb: error: build.ninja:15: expected '=', got identifier
build src/Styles.reast : build_ast_from_re $src_root_dir/src/Styles.re
^ near here
Failed
bsb: error: build.ninja:15: expected '=', got identifier
build src/App.reast : build_ast_from_re $src_root_dir/src/App.re
^ near here
Failed
...
On subsequent runs of yarn bsb -clean-world -make-world, I got this output:
Cleaning... 18 files.
Cleaning... 11 files.
bsb: error: loading 'build.ninja': No such file or directory
Failed
Cleaning... 0 files.
bsb: [15/15] src/Json.cmj
bsb: [18/18] Json_encode.cmti
bsb: [8/8] src/atdgen_codec_runtime.cmj
bsb: [10/10] atdgen_codec_runtime.cmti
File "bsconfig.json", line 5:
Error: Unsupported jsx version 2
For more details, please checkout the schema http://bucklescript.github.io/bucklescript/docson/#build-schema.json
error Command failed with exit code 2.
From the output, it is unclear which dependency has the unsupported JSX version (atdgen_codec_runtime is actually the last successfully compiled one).
Also, the URL from the error message (http://bucklescript.github.io/bucklescript/docson/#build-schema.json) gives ā404 Not Foundā.
When accidentally running yarn bsb -clean-world -make-world in the root folder of the monorepo (which does not currently have a bsconfig.json, only the subfolders have one), I got the error message
File "<my project root>/bsconfig.json", line 1
Error: Invalid json format
error Command failed with exit code 2.
Instead of āInvalid json formatā, it should say āFile not foundā.
Hi @cknitt, thanks for the feedback, this is very helpful.
yarn install took an eternity for some reason
I guess you are testing it on a Mac where I did not make a prebuilt binary
On the first run of yarn bsb -clean-world -make-world, I got the following errors:
This is that you are running an old version of bs-platform which generates some old syntax of build instructions, a second build will get rid of them ā to avoid such confusion, I made some changes to support both
From the output, it is unclear which dependency has the unsupported JSX version
Now we print a log when bsb is building a dependency
Thanks to the feedback from @cknitt, I made a second release for testing 8.3.3-dev.2 (for Mac/Linux platform)
Some design decisions:
We can only support one entry point right now, since some configs have only one entry point, notably package-specs, we get from the toplevel package specs that whether its dependencies should be using es6/commonjs etc.
We need some user input to tell which packages are pinned packages, I am thinking of introducing an extra file, pinned-packages.json so that bsb will run generator rules for pinned packages.
There will be three kinds of packages
toplevel packages (decide which output format to be used, dev dependency/generator rules always run, warnings on )
@Hongbo Would it be possible to add a new property pinned-dependencies to bsconfig.json file? That would avoid the need to add new config files (there are already too many in JavaScript and OCaml imho) and it would also be consistent with other fields like bs-dependencies.
Another alternative is to allow for objects inside bs-dependencies, e.g.
Yes, I guess. But in my experience, this does no happen very often. You pin, then you develop for some time, then potentially you unpin when changes are merged upstream, if ever. But switching back and forth is not common (in my experience, curious what others think).
Why dont we open an issue where people can report issues?
Anyway, seems like this breaks on a dependency we had(which didnt break before) -> reason-date-fns
(seems like its a problem in the dependency, but still this is a regression)
Regarding the extra config, please not yet another config file, canāt stress this enough, we should think how to remove things, not add. maintaining my dependencies in two places due to reason is already annoying(yarn/npm should be the de-facto place where i define my dependencies). now iāll have to maintain yet another obscure(and itāll be obscureā¦) config file for 1 tool.
We should really really work extra hard on aligning ourselves to how yarn/npm is doing this kind of things. a dependency, if we were to use package.json to determine dependencies(as we definitely should IMHO), this becomes much easier, no?
Ok, pushed forward by manually fixing the reason-date-fns.
Now it just doesnt work:
Iāve package A that depends on package B(Aā>B)
I run make-world on A. it runs fast(not that we care about speed at this point at all).
I change something in B that should break the build(e.g introduce a new argument in a func)
Run make-world again in A -> Everything passes(but i see some different logs)
When i do an actual dev flow, e.g make-world on A than watch on A, it breaks in even worse way(e.g some weird error)
Sidenote:
Can we please stop emphasizing speed? its such a grave mistake. my team and i want documentation, dev-experience improvement(better errors, better tooling etc). Its already fast enough but what good is fast compiler if we spend hours trying to understand weird errors and non deterministic behaviour?
Do you have anything we can reproduce step by step?
Btw: using a hash tone is not constructive, we donāt prioritize things based on the volume of criticism, it probably cause an opposite effect . @cknitt ās feedback is a role model
Regarding reproduction, i dont have anything at hand. but i would think that any simple repo with such constructs will behave this way. happy to do that but itāll take me a bit - lmk if you want me to do that.
So in your proposal, I assume that the pinned package settings only work for toplevel packages?
Is that common that you want to pin a transitive dependency?
Assume that you have packages, a , b ,c and d where a depends on b and c, b and c depend on d. With your pinned setting only b and c can be pinned, d can only be pinned when you add it to the dependency of a, would that cover most needs?
Hi @Hongbo! You are right, I had tested on macOS, thatās why the yarn install took so long.
Thanks a lot for the new test version (8.3.3-dev.2)! I tried it with the same project as before, here are my findings:
If I run bsb in a directory without a bsconfig.json, I now get the correct error message <my_project>/bsconfig.json: No such file or directory.
On the first run of yarn bsb -clean-world -make-world, I got an error because of an incorrect bsconfig.json in one of the dependencies. It seems that previous versions did not check for this? After fixing the dependencyās bsconfig.json, I was able to continue.
File "<my_project>/node_modules/@reason-react-native/push-notification-ios/bsconfig.json", line 2:
Error: package name is expected to be @reason-react-native/push-notification-ios but got @reason-react-native/__template__
For more details, please checkout the schema https://rescript-lang.org/docs/manual/latest/build-configuration-schema
error Command failed with exit code 2.
On the next run of yarn bsb -clean-world -make-world, I still got
bsb: error: build.ninja:15: expected '=', got identifier
build src/Persistence.reast : build_ast_from_re $src_root_dir/src/Persi...
^ near here
Failed
bsb: error: build.ninja:15: expected '=', got identifier
build src/Styles.reast : build_ast_from_re $src_root_dir/src/Styles.re
^ near here
Failed
bsb: error: build.ninja:15: expected '=', got identifier
build src/App.reast : build_ast_from_re $src_root_dir/src/App.re
^ near here
Failed
bsb now outputs which dependencies it is building which is very nice.
Build failed because warning 103 occurred and a dependency had configured -warn-error @a. The error message said %@string is redundant here instead of @bs.string is redundant here:
Warning number 103 (configured as error)
<my_project>/node_modules/@reason-react-native/push-notification-ios/src/ReactNativePushNotificationIOS.re:63:22-70:22
61 ā ~applicationIconBadgeNumber: int=?,
62 ā ~fireDate: Js.Date.t=?,
63 ā ~repeatInterval: [@bs.string] [
64 ā | `minute
. ā ...
69 ā | `year
70 ā ]
71 ā =?,
72 ā unit
FFI warning: %@string is redundant here, you can safely remove it
After fixing this plus several places where top-level statements were not returning unit, everything compiled fine.
@Hongbo I think itās not uncommon to pin a transitive dependency.
To show a real world example, I took Ahrefs monorepo and created a simplified version of the dependencies between packages (right now itās all under one bsconfig, but we keep some fake pseudo-namespacing hoping to go back to pinned deps one day):
I think i missed something, where was āpinnedā concept explained?
But if i get the rational - canāt we use package.json resolution for that?
On the repo situation, its actually very weird and inconsistent.
Itās happening on our monorepo(which is a hafty one, purely reasonml, > 10 packages) but i couldnt reproduce this in a smaller situation. Itās also not consistent.
E.g, some changes in a dependant package are being correctly detected, but some are not.
The weird thing is, i do see the error in the editor(amazing), but the compile succeedsā¦ which is scary.
Iāll spend some more time trying to reproduce it in a smaller repo, but not sure what to do if that doesnt succeed.
As this introduces a reliability issue to the compiler, i think itās worth to get to the bottom of this(i really hope iām not doing anything stupid hereā¦ but i triple checked myself and everything looks legit)
