What do you think?
9 │
10 │ let cloneInTemp = (temp: string): string => {
11 │ **cd(temp)**
12 │ exec("git clone git@github.com:myorg/myrepo.git")
13 │ }
The return value of the highlighted expression is not used.
Assign the value to a variable, or pass it to the `ignore` function to silence the error.
Example:
let variable = expression
expression->ignore
Since this thread already started a wider discussion, I wanted to state:
I always prefer to use the pattern let _nameOfUnusedValue = <domeSomethingWhichValueWillBeIgnored> over using ignore.
On several occasions, we needed to debug for some time just to find out, an ignore statement (often used by more inexperienced colleagues) “swallowed” an unintentionally partially applied function. Either due to an incorrect usage of the function in the first place or missing adaptions after refactoring.
Having the expression written without ignore at least simplifies the verification to just hover over it in the editor.
Worth noting: It’s always good to think about why you would intenionally ignore a value and what this should mean to your application logic. E.g. are you missing some error handling? Are you having the right assumptions?
In many cases this error just points to the fact that your logic is incomplete.
In this threads example cd eventually could fail. To make the code more resiliant, the returned string should be evaluated if it represents success/failure and e.g. either throw an exception or return unit.
To summarize: I think this is an important error to report, although I’m not quite sure on how to pack all the possible implications into an easy to understand error message for everybody.
I don’t know if ignore is idiomatic, but I certainly don’t think it should be. At the very least I would prefer the suggestion of let _ = ... over ignore, since that can more easily be annotated with a type to prevent accidental partial application.
But mostly I just think bad APIs ought to be fixed (looking at you Promise 
)
I actually think that’d be helpful here. The issue for me, specifically was “somewhere wanted unit”, but the somewhere wasn’t defined, which makes me want to debug in other areas.
I think you’re correct. The actual code is wrapped in a try block to catch the JS errors that may pop from using shelljs (the library containing cd). It is incomplete logic! The thing with js interop for a beginner here is the js method cd returns a cwd, the method will return a string and not a null nor undefined value, but we’re saying, in this case, the method is expected to return unit, but when in other contexts, the same method’s external signature may be string depending on the needs of the file, which I think is a bit of trouble to navigate when you’re interacting with libraries. Either you put a much larger interface over the js interop, or you have wonky bindings.
One other thing that I might be interested in (at least to the above), isn’t this how the semicolon would work in ocaml? I mean, practically it just indicates to discard the left value as a side effect (which, in essence, is how the expressions work in my example). Now, that’d be confusing as hell for a js dev to sometimes use semicolons, but just another way here if I remember correctly.
You’re right. There are roughly 4 categories of bindings / libs:
cd returning a string) Users of such libs/bindings need to do their homework on how to safely interact with suchexternals)Since performance and bundle-size seem a common concern in the rescript community many (if not most?) libs try to take the first or second approach.
I find myself using approach 3 most of the time. - I don’t care (and don’t need to) for a minimal perf impact, if I can get every day life improvements out of it.
If you’d try to achieve your task in pure js, you still would need to verify the result of cd, if you care about correctness and defensive programming. The only difference in rescript is, that it becomes more obvious if you don’t.
Therefore, I would include the validation of the result in the binding-lib, if I’d have to write them myself. Probably providing an interface like let cd: string -> Belt.Result.t(unit, string) or similar.
I guess you already knew, but just wrapping the whole block with a try statement won’t help in certain cases: e.g. cd fails because dir doesn’t exist - due to no validation, program continues - next operation gets executed (maybe some file operations? e.g. create some files?) - now some invariants of your program are violated, but you wouldn’t get an exception
Regarding the ocaml syntax. I’m absolutely not well versed at ocaml and therefore not comfortable answering your question. But I believe, semicolons are not ment to just disregard (ignore) any expression.