rescript-lang/rescript

Source of truth for standard library types

Opened this issue · 23 comments

cknitt commented

Currently we have e.g. in (Core) Map.res:

type t<'k, 'v> = Js.Map.t<'k, 'v>

or in (Core) Null.res:

@unboxed
type t<'a> = Js.Null.t<'a> =
  | Value('a)
  | @as(null) Null

i.e., the Js modules are the source of truth.

This should be changed, as Js should be deprecated.

Two options:

  1. Invert this to make the Core modules the source of truth, i.e., Js.Map.t would refer to Map.t.
  2. Create a new module Runtime_types.res, Types.res or similar, collect all the types there and make it the source of truth.

Any opinions @cristianoc @zth @cometkim?

cometkim commented

For null and undefined, Primitive_js_extern is the source of truth right now

Core -> Js -> Primitive

cometkim commented

My theory here is that Js is more like core, unless we assume that we can add more backend other than JS later.

Core is not really "core" because it has a lot of host dependencies. (Js, Html, Intl, etc)

cknitt commented

The way I perceive it is that Js is the legacy stdlib and Core (although the name is actually not user-visible anymore) the new stdlib.

Primitive_js_extern has more than the types themselves though.
And the question is what will the user see in hover help etc. "Primitive_js_extern" does not sound very user-friendly.

cknitt commented

The way I prototyped it in https://github.com/rescript-lang/experimental-rescript-stdlib-build/blob/main/runtime/src/runtime_types.res, I had

Core -> Runtime_types
Js -> Runtime_types
Belt -> Runtime_types

cristianoc commented

Another question is the module name, if any.

Should it be Map.t or just map.

cristianoc commented

Same question for the existing built in types.
Which ones should have their own module name.

cometkim commented

Should it be Map.t or just map.

Js maps are not transparent in their implementation. I would never recommend treating them as primitives.

cknitt commented

It would certainly be nice to have date instead of Date.t, regExp instead of RegExp.t and symbol instead of Symbol.t etc.
Like we have dict instead of Dict.t now.

cometkim commented

It would certainly be nice to have date instead of Date.t, regExp instead of RegExp.t and symbol instead of Symbol.t etc.

As I mentioned in Discord before, Date is poorly designed and soon to be replaced by Temporal

Array and list structures are simple and predictable, but map/set have much wider use cases. Whether it is a tree or a hashtable should be determined at the call site, and the only thing that the build depends on is the abstract type.

Dict is something that we already had in other forms.

Symbol is a primitive type in JS, so it is worth considering, but RegExp is just an Object with a different prototype.

IMHO we have to be very careful when choosing these. Treating something that isn't primitive as if it were a primitive is a decision we can never reverse. And it limits the quality of the implementation beyond just the interface.

cometkim commented

Do you think Belt is no longer needed once Core bindings are provided? In fact, there are examples where Belt Map works more efficiently than Js Map.

https://github.com/cometkim/benchmark-rescript-cache-impl

We don't have to specify the properties of implementations in the language, even if Js does. I keep emphasizing the different responsibilities of primitives and stdlib.

DZakh commented

As I mentioned in Discord before, Date is poorly designed and soon to be replaced by Temporal

I disagree that it should be the reason not to include the date type. It's a standard of the platform, and even though it's poorly designed, it's still a primitive which is widely used.

  • 👍1
cometkim commented

I am not against adding widely used utilities to the standard library.

However, "widely used" does not imply the existence of primitives. Primitives are the most fundamental part of a language and have a lasting impact on subsequent designs. Beyond simply declaring built-in types, they interact with other features of the language, adding complexity and side effects of its semantic. Even with poorly designed one, they are more cumbersome to work with and lower the average quality of software written in ReScript. Especially Date has never been progressively enhanced in ECMAScript specification. In fact, many codebases are trying to avoid relying on it and use third-party libraries like moment.js or dayjs.

There are reasons why stuffs like Date and RegExp are not chosen as primitives in other languages ​​(except Raku), despite their obvious popularity.

cometkim commented

If pattern match is possible for arbitrary constructors like Date or RegExp, I would ask why it is not supported for all other constructors. In most codebases, there are custom constructors that are obviously used more frequently than those.

cometkim commented

If we support Date more conveniently because it already exists, then that is literally why people use Date. Because it is better supported, users avoid better alternatives than Date, even if they are standards driven.

This is exactly what happened between CommonJS and ESM. ESM offers a better future, but CommonJS didn't switch simply because it was work and advanced use cases.

The interoperability story is not simple, because the two specifications have distinctly different semantics. The same thing probably happens between Date and Temporal.

cknitt commented

I was not saying that we need to make date and regExp built-in types (as in https://github.com/rescript-lang/rescript-compiler/blob/master/compiler/ml/predef.ml) and implement pattern matching for them.

This is just about the fact that the "source of truth" cannot stay in the Js modules as these are legacy modules that will eventually be moved out to some compatibility package, and we need to decide where to put them instead.

The easiest solution without any additional modules or circular dependencies:

Date.res:

type t

Js_date.res:

type t = Js.Date.t

and, for convenience, Pervasives.res:

type date = Date.t

Also not saying that we necessarily need to have aliases for Map.t and Set.t in Pervasives.

When Temporal is added later, I see no harm in having both

type date = Date.t
type temporal = Temporal.t

in Pervasives.

zth commented

I was not saying that we need to make date and regExp built-in types (as in https://github.com/rescript-lang/rescript-compiler/blob/master/compiler/ml/predef.ml) and implement pattern matching for them.

This is just about the fact that the "source of truth" cannot stay in the Js modules as these are legacy modules that will eventually be moved out to some compatibility package, and we need to decide where to put them instead.

The easiest solution without any additional modules or circular dependencies:

Date.res:

type t

Js_date.res:

type t = Js.Date.t

and, for convenience, Pervasives.res:

type date = Date.t

Also not saying that we necessarily need to have aliases for Map.t and Set.t in Pervasives.

When Temporal is added later, I see no harm in having both

type date = Date.t
type temporal = Temporal.t

in Pervasives.

This sounds like the best decision to me. I know it's not what you meant @cknitt , but just for the record, I think @cometkim has a good point about what to make builtins and not (which I also think we all agree on already). But just aliasing is fine.

  • 👍1
cristianoc commented
  • 👍2
cometkim commented

The important difference, I think, is whether it is treated as something the compiler can depend on.

zth commented

The important difference, I think, is whether it is treated as something the compiler can depend on.

There are exceptions to this though. Regexp and Date are already special cased in unboxed variants, when though they're not defined in the compiler per se.

cometkim commented

There are exceptions to this though. Regexp and Date are already special cased in unboxed variants, when though they're not defined in the compiler per se.

That's the concern. Some things go beyond declaring types and going to be deeply integrated into the compiler.

cometkim commented

I had the same thought when moving null and undefined types to Primitive_js_extern module. If possible, it should not be integrated too deeply, because in ReScript, option is our primitive, and null and undefined are just details of the host language.

What does that translate to if we support WASM targets in the future? I'm not trying to expand the topic we're discussing now, but it's about the direction it could take as we design the language.

cknitt commented

I was not saying that we need to make date and regExp built-in types (as in https://github.com/rescript-lang/rescript-compiler/blob/master/compiler/ml/predef.ml) and implement pattern matching for them.
This is just about the fact that the "source of truth" cannot stay in the Js modules as these are legacy modules that will eventually be moved out to some compatibility package, and we need to decide where to put them instead.
The easiest solution without any additional modules or circular dependencies:
Date.res:

type t

Js_date.res:

type t = Js.Date.t

and, for convenience, Pervasives.res:

type date = Date.t

Also not saying that we necessarily need to have aliases for Map.t and Set.t in Pervasives.
When Temporal is added later, I see no harm in having both

type date = Date.t
type temporal = Temporal.t

in Pervasives.

This sounds like the best decision to me. I know it's not what you meant @cknitt , but just for the record, I think @cometkim has a good point about what to make builtins and not (which I also think we all agree on already). But just aliasing is fine.

There is a problem with this approach though. If my project also has a Date.res (like, for example in gentype_tests/typescript-react-example), weird things start to happen like

  It's possible that your build is stale.
  Try to clean the artifacts and build again?

  Here's the original error message
  The files /Users/christoph/projects/cca/rescript-compiler/darwinarm64/../lib/ocaml/pervasives.cmi
  and src/date.cmi make inconsistent assumptions over interface Date

(BTW notice that this error message refers to the non-existing path src/date.cmi) or

  The module or file Date can't be found.
  - If it's a third-party dependency:
    - Did you add it to the "bs-dependencies" or "bs-dev-dependencies" in bsconfig.json?
  - Did you include the file's directory to the "sources" in bsconfig.json?

So it might be better if those types were defined in a different file with a name less likely to occur in user projects.

cknitt commented

This way works fine for me: 5828460