An innovative workflow for warnings/errors handling in 8.3.0-dev.1

Merlin is never reliable. Imagine if you don’t touch the file, Merlin has no idea if there are something wrong with the file.
“Respond to every user” this model won’t work , it would essentially make the OSS maintainer exhausted, sometimes you have to trust the maintainer’s judgement.
I care about users feedback, that is why I am still reply at 11pm in my timezone but this model is not going to be sustainable for my mental health

1 Like

Yes, there won’t be warning any more like golang. Everything is an error but user can turn off some non critical errors. This matches your expectation.
I will think about if we can improve the user experience further

1 Like

I don’t want to take your time with this meta-discussion, but I think you are just interpreting some responses (including mine) incorrectly. You don’t have to listen to anyone at all. You even don’t have to put things up for discussion. But in my opinion if you put things up for discussion and users have a number one concern (correct me if I am wrong but I think the thing we are discussing was the number one concern). It’s a bit frustrating for the people who have tried to assist you with the feedback (also sometimes at 11PM in their free time) to see that number one concern not addressed (I mean have a response to that concern, not perse that a solution is offered).

Anyway. I just wanna say that I think Rescript is amazing. And really thank you for your work for creating this. I am not a disgruntled user at all and I am not looking to demand anything. I am just trying to explain my arguments with quite a lot of production code under my belt.

2 Likes

I understand, and this workflow is perfect for me personally. However I guess others like to have non-fatal warnings which they can fix after finishing up a hacking session :slight_smile:

Thanks for thinking deeply about this.

1 Like

This doesn’t match the expectation in my opinion. Unused variables (an example of a frequently occurring warning) is a great warning and it serves a great purpose. Usually you do have some iteration steps where that warning is not important to fix just yet. But you really want to get these unused variables out before you commit. Changing the bsconfig.json is not a solution to that.

Is there a technical reason not to emit JS when there are soft-errors? And can these reasons be overcome? Or is it more of a principal/philosophical argument to not do this? If it’s the latter it might make sense to talk about this a little more to end up with a well thought out solution that would work for most people. (Or maybe there is something wrong with our process in needing warnings and we should change our approach?). I understand that it feels exhausting, and I will be careful of my tone, but I think it’s a good discussion to have (this is really impacting productivity so it doesn’t feel like bikeshedding).

I will think more deeply in how often I need warnings that don’t block compile, it feels often in UI development, but maybe I am mistaken. I will try the new release and see the real impact on productivity.

4 Likes

Yes and golang also gets criticism for it: https://stackoverflow.com/questions/21743841/how-to-avoid-annoying-error-declared-and-not-used (see especially the last comments (not answers) to the question).

Not sure if ReScript should follow suit there. I am sure there is some middle ground to explore.

1 Like

There were also some attempts at doing with eslint in JS in popular presets (I think the Airbnb preset was the most popular one). And in the end most people found that it was hurting productivity moving away from those presets that had all warnings triggering errors.

By the way. I’d like to say that for some use-cases I think this option makes sense. In graphql-ppx (a compiler) I have warnings-as-errors turned on. It only compiles when all warnings are addressed, and I have no problems with it for this use-case. But for the UI-heavy app I am working on on a daily basis I THINK this would be quite frustrating and less productive.

1 Like

By the way. This reminds me of an issue I do run into quite a lot. Would there be a way to have deterministic errors? Currently if there are a LOT of errors, every compile the error being shown (or being shown last) is non-deterministic, so the feedback loop is not great. When I try to fix the error and recompile then I am being shown a different error irregardless if the previous error was fixed or not.

4 Likes

This is exactly the problem we are trying to solve.
With reliable editor tooling, the error is displayed in line, so user does not need to look at the command line output, the error will be properly displayed inline. More important, it’s reliable unlike merlin or other 3rd party diagnostics tools.

1 Like

The fact that JS files are not generated anymore for files with warnings makes the developer experience significantly worse than before.

The thing that it’s handled this way is that you need tell build system there’s some failures for this module so it will do the rebuilt next time to collect the same set of errors. Otherwise, it’s unclear how build system will do the rebuild

Is it not possible to have both - reliable editor tooling AND - like in all previous BS versions - JS still being emitted for files with warnings?

So far I think you have to pick one, that’s a trade off we need make. Maybe we can provide an option like ignore non critical errors. In this case, by default, you get reliable editor tooling, if you explicitly opt-out, you are on your own, is this a solution to you?

An opt-out that restores the current (pre-8.3) behavior would certainly be helpful - especially in the current situation where the new reliable editor tooling is not even available yet and we need to keep using Reason syntax and RLS for some time. Thank you for considering this!

3 Likes

However, I still do not believe that the new approach is the right one even for new users coming from JS.

Most users coming from JS will be using ReScript to build React apps. They will already be used to the tight feedback loop of saving changes in the editor and having the UI refresh more or less instantly as long as there are no syntax errors in the code so that babel can still compile it. If there are any warnings (detected by eslint) these will be shown in the editor, but they will not prevent the UI refresh/feedback loop.

So this behavior is what people will expect DX-wise (and what used to work in previous BS versions, albeit not with reliable error reporting in the editor tooling).

In short, I believe what we are talking about is not some use case that only @jfrolich and I have, but actually the main use case for ReScript users, and I think we should be careful not to break it.

2 Likes

here’s a complaint from new user who noticed partial match does not stop compiling https://github.com/rescript-lang/rescript-compiler/issues/4646
I am going to stop here, since we all understand the trade off clearly