This is great, thank you! It would make interop so much more comfortable.
I’m likely overcomplicating, but can’t it lead to a religious split in how people design their constructors in the ReScript land?
// Make a thing like this:
let myThing = Superlib.Thing.make("Bowser", {
setting: "Mario",
level: "Castle",
})
// versus
let myThing = Superlib.Thing.make("Bowser", ~setting="Mario", ~level="Castle", ())
An explicit and clear note about idiomatic way in the docs would help a lot.
UPD: You did the strong move for the fast-pipe vs pipe-last and the trailing unit arg saving hours of discussion. I see here the similar situation which requires a strong opinion from the BDFL.
There is no one size fits all solution in my opinion.
For function with around 3~4 optional arguments, the labeled ones seems to be more efficient.
But there are cases with 10 or even more optional arguments, you don’t want a function to have that many arguments, the object one makes more sense
Something like that would make so much more sense.
Using the @obj decorator for a record type is vague enough to imply other intentions. E.g. for me it always feels like it has something to do with OOP related interop.
@stripUndefined, or even better @stripNoneFields, would do exactly what it says, and there’s way less room for wrong expectations. It’s a little more verbose, but considering this feature is being used irregularly for specific cases, I expect it would be fine.
@obj is an overloaded attribute for external functions which does a similar thing, but kinda different (a feature I assume gets less relevant now). We’d need to mix up multiple use-cases for the same attribute on the docs, like in the syntax exploration widget, which IMO makes it a little harder to find the right solution right away.
I think one drawback of names like “strip*” and “compressed” is that they don’t really convey that the user doesn’t need to specify optional fields when instantiating. I guess if regular objects had that same capability, but did emit undefined fields then those names would be better. Maybe that’s the long term goal?
I think this thread mixed up two different topics / features:
Being able to “skip” fields that are of type option<..> … e.g {firstname: "test", lastname: "test"} is possible, even though it’s {firstname: string, lastname: string, age: option<int>}
Being able to control the output by adding a stripNoneFields / @compressed to prevent undefined definitions in JS objects (such as {firstname: "test", lastname: "test", age: undefined})
That means, 1) would always apply, no matter if you annotate it or not. Or am I misunderstanding the actual proposal right now?
The problem with that in my opinion that in most cases you want the compiler to complain if you forgot/not included a field. Only if you have some kind of config object with many optional values the proposed behavior is much nicer.
Perhaps it should be two independent annotations? (Or a syntax like I proposed above where you explicitly signal that you want all the other optional fields to be set to None)
I’d like to mention the positive affect on many newer Web API bindings this RFC would bring.
A common pattern on newer JS proposals is to “init objects”. This is a name I’ve made up to describe a pattern of passing a relatively large option bag as an object to class constructors. Examples include new Request({ body: 'hello' }) from the fetch API and Temporal.PlainTime.from({ hour: 5 }) from Temporal. I’ve taken the name from what TypeScript calls the types in the fetch declarations.
This pattern is even more crucial to Temporal, it uses it for methods too such as plainTime.add({ hours: 5 }).
This design pattern is currently very hard to type in ReScript, the best solution I’ve seen so far is what I call an options bag module:
module AddOptions = {
type t
@obj
external make: (~hours: int=?, ~minutes: int=?, ~seconds: int=?, ()) => t = ""
}
@send external add: (t, AddOptions.t) => t = "add"
plainTime->add(AddOptions.make(~hours=5, ()))
This is really not an ideal DX - when using an API like this it takes a long time to figure out “ok this function accepts an AddOptions.t, how do I make one of those?”. It also bloats the bindings a huge amount which is a maintenance burden.
When installing the master branch as rescript you get a bunch of errors in other rescript packages.
For example, in rescript-react you get:
We've found a bug for you!
/Users/mando/Github/project/node_modules/@rescript/react/src/RescriptReactRouter.res:24:57-59
22 │
23 │ @send
24 │ external replaceState: (Dom.history, @as(json`null`) _, @as("") _, ~hre
│ f: string) => unit =
25 │ "replaceState"
26 │
expect int, string literal or json literal {json|text here|json}
FAILED: cannot make progress due to previous errors.
or
FAILED: cannot make progress due to previous errors.
Failure: /Users/mando/Github/project/node_modules/rescript/darwin/ninja.exe
Location: /Users/mando/Github/project/node_modules/@rescript/react/lib/bs
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
~/Github/project demo !18 ?4
❯ yarn watch
yarn run v1.22.10
$ rescript build -with-deps -w
File "bsconfig.json", line 1
Error: package @rescript/react not found or built
- Did you install it?
- If you did, did you run `rescript build -with-deps`?
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
~/Github/rescript-timer-hooks demo !18 ?5 ✘ 2
❯ yarn watch
yarn run v1.22.10
$ rescript build -with-deps -w
Dependency on @ryyppy/rescript-promise
rescript: [1/5] src/Promise.iast
FAILED: src/Promise.iast
We've found a bug for you!
/Users/mando/Github/project/node_modules/@ryyppy/rescript-promise/src/Promise.resi:32:12-57:1
30 │ external reject: exn => t<_> = "reject"
31 │
32 │ @ocaml.doc("
33 │ `make(callback)` creates a new Promise based on a `callback` that recei
│ ves two
. │ ...
56 │ ```
57 │ ")
58 │ @bs.new
59 │ external make: ((@bs.uncurry (. 'a) => unit, (. 'e) => unit) => unit) =
│ > t<'a> = "Promise"
Offset: 260, Invalid escape code: "
rescript: [2/5] src/Promise.ast
I am probably misunderstanding what you mean by breaking change. Are these errors indicative of a breaking change or perhaps that the underlying package needs to be updated?
This is now being considered for the upcoming 10.0 release.
One possibility is to make @obj the default. The reason for this is that, for example, that will be eventually what’s used in JSX v4, when it ships (after 10.0). So it will be used in every component. The less magic one puts in the JSX, the better. Also, it seems that if one wants to give the choice at all, then one case should be the default, and the other one should be reserved for advanced users only. So that someone learning the language does not get entangled with such a distinction. And components (will) already take @obj as the default.
But, there are going to be tradeoffs. So curious to hear thoughts about this.
Perhaps more importantly, could changing the default break existing code? Is there code that relies on the field being there, with value undefined, vs it not being there?
The possibility to distinct between “absent” and undefined data has been introduced in Typescript 4.4. Even if IMHO is a bit silly that two different nil values are not enough (because, in practice, we would translate the pattern to something like option<nullable<'a>>), maybe someone showed some use cases on the issue/PR that led to this Typescript feature. Hopefully there are some interesting discussions that can be useful to better understand tradeoffs.
Is there code that relies on the field being there, with value undefined , vs it not being there?
There most certainly will be.
We don’t need to worry about destructuring: var { x = 1 } = { x: undefined } sets x = 1, so someone has to go through the hassle of Object.hasOwn(obj, 'x').
However, stuff like Object.keys() and co. will yield different results - and I would not be surprised if there’s some poor uses of that in the wild.
I have no strong opinion here, but I’d like to remind that records are not only used as a container for options that are, well, optional by definition. Instead, records are often used to represent a state data type encapsulated within a module.
In such a case, arguably, it’s more reliable to have a compiler that will bark on all the places where a newly introduced field happened to have an option<...> type is missing from the initializer. That is, without @obj behavior by default.
This aspect, of different typing behaviour w.r.t. normal records, is well understood. And surely one needs to evaluate its impact.
Instead, it seems that the dangers of changes in runtime representation are less well understood. It really depends on whether people do make use of the assumption about the data representation where missing values are fields present with undefined value. We know it’s possible. It would be interesting to know whether it’s actually happening in what people do.
These dangers need to be understood no matter what is the default. The only difference is how prevalent those dangers would be.
In case of the options, I think {foo:undefined} is actually more likely to break something than {}.
I can think of only one common use case where this certainly makes a difference: when an object is used as a dictionary. In this case Js.Dict is usually more appropriate on the ReScript side. But even if record is used for some reason, omitting the property is probably the desired behaviour.