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, it is always suggested to report bugs with a reproducible repo, that would be better use of time for both.
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 hereinstead 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.
1 Like
@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):
"a": [],
"b": [],
"c": ["a", "b"],
"d": ["a", "b", "c"],
"e": ["a", "b", "c"],
...
You can see how c is a pinned dep that depends on a, b but it is also a dependency for packages d, e,…
I hope this helps, let me know if we can provide more information.
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)
Ok, i think i got it:
play with Test.re in “b” package(try adding/removing variables) the compiler behaves very weirdly.
I can’t emphasize this enough! Same can be said for emphasizing on syntax instead of semantics. (We have slightly better syntax - at cost of splitting the ecosystem and a worse developer experience for a long time - but we still have a Js.String2 module).
1 Like
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.
Yes, I added a sanity check, glad it profited immediately
bsb: error: build.ninja:15: expected ‘=’, got identifier
Seems I checked in the source changes, but did not prebuild the binary properly on your platform, should be an easy fix
The error message said
%@string is redundant hereinstead of@bs.string is redundant here
It should be @string. We will introduce some more robust mechanism to avoid warn-error in dependencies
1 Like
If you want to pin a transitive dependency, maybe just introduce a field in bsconfig.json?
"pinned-dependencies" : [
]
I do see the error in the editor(amazing), but the compile succeeds
The error diagnostics were never reliable, that’s why we work on capture the warnings in the rebuild, the benefit will trickle down in the editor in the future.
@Hongbo did you manage to reproduce?
I see you renamed my monorepo issue to pinned deps without explanation of what it is… please note that i cant guess what pinned deps are 
@Hongbo yes, this is what I was suggesting in a previous comment:
I am not sure I understand if transitive and direct dependencies should be treated differently. Both could be defined in the top level package bsconfig.json, right?
By the way, there is a related feature in Yarn called “resolutions”, it might be worth to check the documentation and original RFC. It is maybe too complex for the purposes of pinned dependencies, but it might serve as inspiration.
@BlueHotDog the pinned packages solution was explained briefly here. The idea is to solve the monorepo issues by telling bsb which packages are being used “from source”, so:
- installation of these packages is skipped
- changes are always picked up
Thanks @jchavarri!
I’ll add a link to the Issue, as i think the issues should be ReScript source of truth and not the forum.
But it’s great to see such a detailed explanation.
And yes. if the whole point of the community rift is to align better with JS(and provide better DX), IMO yarn resolutions is a much better solution(which we already use extensively in our projects, and works, so i’m not sure there’s even an issue) vs adding another field people will need to learn(plus document, maintain etc etc etc)
1 Like
I understand that there are some duplications between bsconfig.json and package.json. The reason is that we don’t want to be tied to a specific package manager, in the future, it may make room for our own package manager
@Hongbo, using the file format isnt the same as being tied to yarn/npm(e.g there are even already 3 package managers on top of package.json)… i just think that if the priority is DX (as it should be IMO) -> forcing duplication for some ambiguous reason isnt a good idea IMO.
The pinned package support is finished https://github.com/rescript-lang/rescript-compiler/pull/4826, will make a release for testing!
3 Likes
How will pinned dependencies work in monorepo?
Currently i’ve a resolution flag in the root package.json.
Now i’ll need to copy that to all the various bsconfigs i’ve? what happens if i dont?
Could it be possible to also have a regex-like syntax there? so we can do:
“@orgname/**, !@orgname/comes-from-package-manager”
this should be pretty easy to add once we have a solid implementation