genType: treat option<t> as t | undefined and remove special treatment of record fields of option type.

CHANGELOG.md

@@ -40,6 +40,9 @@ subset of the arguments, and return a curried type with the remaining ones https
 Also, `(. int) => string => bool` is not equivalen to `(. int, string) => bool` anymore.
 These are only breaking changes for unformatted code.
 - Exponentiation operator `**` is now right-associative. `2. ** 3. ** 2.` now compile to `Math.pow(2, Math.pow(3, 2))` and not anymore `Math.pow(Math.pow(2, 3), 2)`. Parentheses can be used to change precedence.
+- `genType`: streamline the treatment of optionals as undefined https://github.com/rescript-lang/rescript-compiler/pull/6022
+  - Represent `option<t>` as `undefined | t` instead of `null | undefined | t`. This is more permissive when importing functions taking optional values (allows to use option types), but stricter when e.g. exporting ReScript functions taking arguments of option type. Fallback: use `Js.undefined<_>` instead.
+  - Represent `{x:option<string>}` as `{x:(undefined | string)}` instead of `{x?: string}`. This is more in line with TS's behaviour. Fallback: use `{x?:string}`.
 
 #### :bug: Bug Fix
 

jscomp/gentype/Converter.ml

@@ -5,7 +5,6 @@ type t =
   | CircularC of string * t
   | FunctionC of functionC
   | IdentC
-  | NullableC of t
   | ObjectC of fieldsC
   | OptionC of t
   | PromiseC of t
@@ -67,7 +66,6 @@ let rec toString converter =
       |> String.concat ", ")
     ^ " -> " ^ toString retConverter ^ ")"
   | IdentC -> "id"
-  | NullableC c -> "nullable(" ^ toString c ^ ")"
   | ObjectC fieldsC ->
     let dot =
       match converter with
@@ -119,8 +117,7 @@ let typeGetConverterNormalized ~config ~inline ~lookupId ~typeNameIsInterface
     | Array (t, mutable_) ->
       let tConverter, tNormalized = t |> visit ~visited in
       (ArrayC tConverter, Array (tNormalized, mutable_))
-    | Dict _ ->
-      (IdentC, normalized_)
+    | Dict _ -> (IdentC, normalized_)
     | Function
         ({argTypes; componentName; retType; typeVars; uncurried} as function_)
       ->
@@ -190,10 +187,10 @@ let typeGetConverterNormalized ~config ~inline ~lookupId ~typeNameIsInterface
           else (IdentC, normalized_))
     | Null t ->
       let tConverter, tNormalized = t |> visit ~visited in
-      (NullableC tConverter, Null tNormalized)
+      (OptionC tConverter, Null tNormalized)
     | Nullable t ->
       let tConverter, tNormalized = t |> visit ~visited in
-      (NullableC tConverter, Nullable tNormalized)
+      (OptionC tConverter, Nullable tNormalized)
     | Object (closedFlag, fields) ->
       let fieldsConverted =
         fields
@@ -227,9 +224,6 @@ let typeGetConverterNormalized ~config ~inline ~lookupId ~typeNameIsInterface
       in
       (TupleC innerConversions, Tuple normalizedList)
     | TypeVar _ -> (IdentC, normalized_)
-    | Undefined t ->
-      let tConverter, tNormalized = t |> visit ~visited in
-      (NullableC tConverter, Undefined tNormalized)
     | Variant variant ->
       let allowUnboxed = not variant.polymorphic in
       let withPayloads, normalized, unboxed =
@@ -363,7 +357,6 @@ let rec converterIsIdentity ~config ~toJS converter =
                 argConverter |> converterIsIdentity ~config ~toJS:(not toJS)
               | GroupConverter _ -> false)
   | IdentC -> true
-  | NullableC c -> c |> converterIsIdentity ~config ~toJS
   | ObjectC fieldsC ->
     fieldsC
     |> List.for_all (fun {lblJS; lblRE; c} ->
@@ -372,7 +365,7 @@ let rec converterIsIdentity ~config ~toJS converter =
            match c with
            | OptionC c1 -> c1 |> converterIsIdentity ~config ~toJS
            | _ -> c |> converterIsIdentity ~config ~toJS)
-  | OptionC c -> if toJS then c |> converterIsIdentity ~config ~toJS else false
+  | OptionC c -> c |> converterIsIdentity ~config ~toJS
   | PromiseC c -> c |> converterIsIdentity ~config ~toJS
   | TupleC innerTypesC ->
     innerTypesC |> List.for_all (converterIsIdentity ~config ~toJS)
@@ -501,13 +494,6 @@ let rec apply ~config ~converter ~indent ~nameGen ~toJS ~variantTables value =
     EmitText.funDef ~bodyArgs ~functionName:componentName ~funParams ~indent
       ~mkBody ~typeVars
   | IdentC -> value
-  | NullableC c ->
-    EmitText.parens
-      [
-        value ^ " == null ? " ^ value ^ " : "
-        ^ (value
-          |> apply ~config ~converter:c ~indent ~nameGen ~toJS ~variantTables);
-      ]
   | ObjectC fieldsC ->
     let simplifyFieldConverted fieldConverter =
       match fieldConverter with
@@ -536,22 +522,12 @@ let rec apply ~config ~converter ~indent ~nameGen ~toJS ~variantTables value =
     in
     "{" ^ fieldValues ^ "}"
   | OptionC c ->
-    if toJS then
-      EmitText.parens
-        [
-          value ^ " == null ? " ^ value ^ " : "
-          ^ (value
-            |> apply ~config ~converter:c ~indent ~nameGen ~toJS ~variantTables
-            );
-        ]
-    else
-      EmitText.parens
-        [
-          value ^ " == null ? undefined : "
-          ^ (value
-            |> apply ~config ~converter:c ~indent ~nameGen ~toJS ~variantTables
-            );
-        ]
+    EmitText.parens
+      [
+        value ^ " == null ? " ^ value ^ " : "
+        ^ (value
+          |> apply ~config ~converter:c ~indent ~nameGen ~toJS ~variantTables);
+      ]
   | PromiseC c ->
     let x = "$promise" |> EmitText.name ~nameGen in
     value ^ ".then(function _element("

jscomp/gentype/EmitJs.ml

@@ -626,7 +626,7 @@ let propagateAnnotationToSubTypes ~codeItems (typeMap : CodeItem.exportTypeMap)
         retType |> visit
       | GroupOfLabeledArgs fields | Object (_, fields) ->
         fields |> List.iter (fun {type_} -> type_ |> visit)
-      | Option t | Null t | Nullable t | Promise t | Undefined t -> t |> visit
+      | Option t | Null t | Nullable t | Promise t -> t |> visit
       | Tuple innerTypes -> innerTypes |> List.iter visit
       | TypeVar _ -> ()
       | Variant {inherits; payloads} ->

jscomp/gentype/EmitType.ml

@@ -140,7 +140,7 @@ let rec renderType ~(config : Config.t) ?(indent = None) ~typeNameIsInterface
     "(null | "
     ^ (type_ |> renderType ~config ~indent ~typeNameIsInterface ~inFunType)
     ^ ")"
-  | Nullable type_ | Option type_ ->
+  | Nullable type_ ->
     let useParens x =
       match type_ with
       | Function _ | Variant _ -> EmitText.parens [x]
@@ -150,6 +150,16 @@ let rec renderType ~(config : Config.t) ?(indent = None) ~typeNameIsInterface
     ^ useParens
         (type_ |> renderType ~config ~indent ~typeNameIsInterface ~inFunType)
     ^ ")"
+  | Option type_ ->
+    let useParens x =
+      match type_ with
+      | Function _ | Variant _ -> EmitText.parens [x]
+      | _ -> x
+    in
+    "(undefined | "
+    ^ useParens
+        (type_ |> renderType ~config ~indent ~typeNameIsInterface ~inFunType)
+    ^ ")"
   | Promise type_ ->
     "Promise" ^ "<"
     ^ (type_ |> renderType ~config ~indent ~typeNameIsInterface ~inFunType)
@@ -161,10 +171,6 @@ let rec renderType ~(config : Config.t) ?(indent = None) ~typeNameIsInterface
       |> String.concat ", ")
     ^ "]"
   | TypeVar s -> s
-  | Undefined type_ ->
-    "(undefined | "
-    ^ (type_ |> renderType ~config ~indent ~typeNameIsInterface ~inFunType)
-    ^ ")"
   | Variant {inherits; noPayloads; payloads; polymorphic; unboxed} ->
     let inheritsRendered =
       inherits

jscomp/gentype/GenTypeCommon.ml

@@ -70,7 +70,6 @@ type type_ =
   | Promise of type_
   | Tuple of type_ list
   | TypeVar of string
-  | Undefined of type_
   | Variant of variant
 
 and fields = field list
@@ -120,7 +119,6 @@ let typeIsObject type_ =
   | Promise _ -> true
   | Tuple _ -> true
   | TypeVar _ -> false
-  | Undefined _ -> false
   | Variant _ -> false
 
 type label = Nolabel | Label of string | OptLabel of string

jscomp/gentype/TranslateSignatureFromTypes.ml

@@ -12,8 +12,9 @@ let translateTypeDeclarationFromTypes ~config ~outputFileRelative ~resolver
     Log_.item "Translate Types.type_declaration %s\n" typeName;
   let declarationKind =
     match type_kind with
-    | Type_record (labelDeclarations, _) ->
-      TranslateTypeDeclarations.RecordDeclarationFromTypes labelDeclarations
+    | Type_record (labelDeclarations, recordRepresentation) ->
+      TranslateTypeDeclarations.RecordDeclarationFromTypes
+        (labelDeclarations, recordRepresentation)
     | Type_variant constructorDeclarations
       when not
              (TranslateTypeDeclarations.hasSomeGADTLeaf constructorDeclarations)

jscomp/gentype/TranslateTypeDeclarations.ml

@@ -1,7 +1,8 @@
 open GenTypeCommon
 
 type declarationKind =
-  | RecordDeclarationFromTypes of Types.label_declaration list
+  | RecordDeclarationFromTypes of
+      Types.label_declaration list * Types.record_representation
   | GeneralDeclaration of Typedtree.core_type option
   | GeneralDeclarationFromTypes of Types.type_expr option
       (** As the above, but from Types not Typedtree *)
@@ -78,7 +79,12 @@ let traslateDeclarationKind ~config ~loc ~outputFileRelative ~resolver
     in
     {CodeItem.importTypes; exportFromTypeDeclaration}
   in
-  let translateLabelDeclarations labelDeclarations =
+  let translateLabelDeclarations ~recordRepresentation labelDeclarations =
+    let isOptional l =
+      match recordRepresentation with
+      | Types.Record_optional_labels lbls -> List.mem l lbls
+      | _ -> false
+    in
     let fieldTranslations =
       labelDeclarations
       |> List.map (fun {Types.ld_id; ld_mutable; ld_type; ld_attributes} ->
@@ -111,7 +117,7 @@ let traslateDeclarationKind ~config ~loc ~outputFileRelative ~resolver
            ->
              let optional, type1 =
                match type_ with
-               | Option type1 -> (Optional, type1)
+               | Option type1 when isOptional nameRE -> (Optional, type1)
                | _ -> (Mandatory, type_)
              in
              {mutable_; nameJS; nameRE; optional; type_ = type1})
@@ -196,9 +202,10 @@ let traslateDeclarationKind ~config ~loc ~outputFileRelative ~resolver
     in
     {translation with type_} |> handleGeneralDeclaration
     |> returnTypeDeclaration
-  | RecordDeclarationFromTypes labelDeclarations, None ->
+  | RecordDeclarationFromTypes (labelDeclarations, recordRepresentation), None
+    ->
     let {TranslateTypeExprFromTypes.dependencies; type_} =
-      labelDeclarations |> translateLabelDeclarations
+      labelDeclarations |> translateLabelDeclarations ~recordRepresentation
     in
     let importTypes =
       dependencies
@@ -227,7 +234,11 @@ let traslateDeclarationKind ~config ~loc ~outputFileRelative ~resolver
                  |> TranslateTypeExprFromTypes.translateTypeExprsFromTypes
                       ~config ~typeEnv
                | Cstr_record labelDeclarations ->
-                 [labelDeclarations |> translateLabelDeclarations]
+                 [
+                   labelDeclarations
+                   |> translateLabelDeclarations
+                        ~recordRepresentation:Types.Record_regular;
+                 ]
              in
              let inlineRecord =
                match constructorArgs with
@@ -342,8 +353,8 @@ let translateTypeDeclaration ~config ~outputFileRelative ~recursive ~resolver
   in
   let declarationKind =
     match typ_type.type_kind with
-    | Type_record (labelDeclarations, _) ->
-      RecordDeclarationFromTypes labelDeclarations
+    | Type_record (labelDeclarations, recordRepresentation) ->
+      RecordDeclarationFromTypes (labelDeclarations, recordRepresentation)
     | Type_variant constructorDeclarations ->
       VariantDeclarationFromTypes constructorDeclarations
     | Type_abstract -> GeneralDeclaration typ_manifest

jscomp/gentype/TranslateTypeExprFromTypes.ml

@@ -230,7 +230,7 @@ let translateConstr ~config ~paramsTranslation ~(path : Path.t) ~typeEnv =
     {paramTranslation with type_ = Option paramTranslation.type_}
   | ( (["Js"; "Undefined"; "t"] | ["Undefined"; "t"] | ["Js"; "undefined"]),
       [paramTranslation] ) ->
-    {paramTranslation with type_ = Undefined paramTranslation.type_}
+    {paramTranslation with type_ = Option paramTranslation.type_}
   | (["Js"; "Null"; "t"] | ["Null"; "t"] | ["Js"; "null"]), [paramTranslation]
     ->
     {paramTranslation with type_ = Null paramTranslation.type_}

jscomp/gentype/TypeVars.ml

@@ -63,7 +63,6 @@ let rec substitute ~f type0 =
     match f s with
     | None -> type0
     | Some type1 -> type1)
-  | Undefined type_ -> Undefined (type_ |> substitute ~f)
   | Variant variant ->
     Variant
       {
@@ -91,7 +90,7 @@ let rec free_ type0 : StringSet.t =
     |> List.fold_left
          (fun s typeArg -> StringSet.union s (typeArg |> free_))
          StringSet.empty
-  | Dict type_ | Null type_ | Nullable type_ | Undefined type_ -> type_ |> free_
+  | Dict type_ | Null type_ | Nullable type_ -> type_ |> free_
   | Option type_ | Promise type_ -> type_ |> free_
   | Tuple innerTypes ->
     innerTypes

jscomp/gentype_tests/typescript-react-example/src/Core.gen.tsx

@@ -2,9 +2,37 @@
 /* eslint-disable import/first */
 
 
-// @ts-ignore: Implicit any on import
-import * as CoreBS__Es6Import from './Core.bs';
-const CoreBS: any = CoreBS__Es6Import;
+import {someFunWithNullThenOptionalArgs as someFunWithNullThenOptionalArgsNotChecked} from './CoreTS';
+
+import {someFunWithNullUndefinedArg as someFunWithNullUndefinedArgNotChecked} from './CoreTS';
+
+const $$toJS552311971: { [key: string]: any } = {"0": "A"};
+
+const $$toRE552311971: { [key: string]: any } = {"A": 0};
+
+// In case of type error, check the type of 'someFunWithNullThenOptionalArgs' in 'Core.res' and './CoreTS'.
+export const someFunWithNullThenOptionalArgsTypeChecked: (_1:(null | string), _2:(undefined | string)) => string = someFunWithNullThenOptionalArgsNotChecked;
+
+// Export 'someFunWithNullThenOptionalArgs' early to allow circular import from the '.bs.js' file.
+export const someFunWithNullThenOptionalArgs: unknown = someFunWithNullThenOptionalArgsTypeChecked as (_1:(null | string), _2:(undefined | string)) => string;
+
+// In case of type error, check the type of 'someFunWithNullUndefinedArg' in 'Core.res' and './CoreTS'.
+export const someFunWithNullUndefinedArgTypeChecked: (_1:(null | undefined | string), _2:number) => string = someFunWithNullUndefinedArgNotChecked;
+
+// Export 'someFunWithNullUndefinedArg' early to allow circular import from the '.bs.js' file.
+export const someFunWithNullUndefinedArg: unknown = someFunWithNullUndefinedArgTypeChecked as (_1:(null | undefined | string), _2:number) => string;
+
+// tslint:disable-next-line:no-var-requires
+const CoreBS = require('./Core.bs');
+
+// tslint:disable-next-line:interface-over-type-literal
+export type variant = "A" | { tag: "B"; value: string };
+
+// tslint:disable-next-line:interface-over-type-literal
+export type t1 = { readonly x?: string };
+
+// tslint:disable-next-line:interface-over-type-literal
+export type t2 = { readonly x: (undefined | string) };
 
 export const null0: (x:(null | number)) => (null | number) = CoreBS.null0;
 
@@ -45,3 +73,14 @@ export const weakmap1: (x:WeakMap<number[],number>) => WeakMap<number[],number>
 export const set1: (x:Set<string>) => Set<string> = CoreBS.set1;
 
 export const weakset1: (x:WeakSet<number[]>) => WeakSet<number[]> = CoreBS.weakset1;
+
+export const option0: (x:(undefined | string)) => (undefined | string) = CoreBS.option0;
+
+export const option1: (x:(undefined | variant)) => (undefined | variant) = function (Arg1: any) {
+  const result = CoreBS.option1((Arg1 == null ? Arg1 : typeof(Arg1) === 'object'
+    ? {TAG: 0, _0:Arg1.value} as any
+    : $$toRE552311971[Arg1]));
+  return (result == null ? result : typeof(result) === 'object'
+    ? {tag:"B", value:result._0}
+    : $$toJS552311971[result])
+};