Deprecate unsafe "j" string interpolation

[cristianoc]

Deprecate unsafe j`$(a)$(b)` interpolation

[glennsl]

What makes it unsafe?

[cristianoc]

What makes it unsafe?

It's untyped: this is compiles OK:

let add = (x,y) => x+y

let v = add(3)

Js.log(j`$(v)`)

Easy to overlook type errors.

[glennsl]

What type error? This doesn't crash, nor will it result in any undefined behaviour or some such. It's no more unsafe than String.make or Json.stringifyAny. Less actually, since the latter will raise an exception for some values. And yet I don't think anyone has argued for removing those. Or even marking them as being unsafe. They're still included in rescript-core, and not marked as unsafe there either.

This doesn't seem to be the real reason...

[cristianoc]

What type error? This doesn't crash, nor will it result in any undefined behaviour or some such. It's no more unsafe than String.make or Json.stringifyAny. Less actually, since the latter will raise an exception for some values. And yet I don't think anyone has argued for removing those. Or even marking them as being unsafe. They're still included in rescript-core, and not marked as unsafe there either.

This doesn't seem to be the real reason...

The type error it does not give, that is. That's the issue. False sense of security, where it's easy to think you're passing a string when you are not.

[glennsl]

The consequence for doing so is extremely benign though. It'll still end up as a string, just not the string that was expected. But that could be said about a lot of features. The unsafety here is on the level of a logic error, and if we're going to start shielding users against that, I'm afraid there won't be much language left.

That's not to say it shouldn't still be removed, I just think this is a bad reason for doing so. And I think it's unwise to dilute the meaning of "unsafe" by applying it in this way.

[glennsl]

What would this say about polymorphic compare, for example? This will happily stringify functions. Polymorphic compare will crash. I still wouldn't say polymorphic compare is "unsafe", in the stricter sense that it goes into undefined territory, but it's certainly less safe than this feature.

[cknitt]

The type error it does not give, that is. That's the issue. False sense of security, where it's easy to think you're passing a string when you are not.

That's the way I see it, too. We were actually bitten by this in production in our (at the time) Reason codebase where we were using this kind of string interpolation to construct some strings shown in the UI, and ended up with undefined showing up in the UI because the value used in the string interpolation was actually of type option<string> instead of string.

This caused me to actually remove all string interpolation from our code at the time and replace it with explicit string concatenations. I am glad that the new ReScript string interpolation actually enforces all values to be strings.

Apart from that, it does not make sense to maintain two slightly different kinds of string interpolation in the language. Also, the j prefix is weird. 😉

[glennsl]

Ok, yeah I buy that. It's error-prone and it's badly designed. To add one more, the way it parses identifiers can sometimes lead to some unexpected results as well.

A better designed feature might have some kind of format specifier to specify the type and toString function, but no rush to implement that.

[TheSpyder]

This is biting me hard in my efforts to upgrade to ReScript 11. Template strings in JS will coerce variables to string, so we didn't see any problem doing the same in ReScript.

It'll probably take me less than an hour to fix, but that's an hour I didn't expect to have to spend on the upgrade.

Arguably the conversion here is more risky than the problems it's avoiding. I have a lot of interpolated strings, and now I need to use regex to find the ones with $var or $(var) in them that no longer emit the interpolated value.