Generic JSX transform completion (#919)

zth · web-flow · commit 5b39b0920757 · 2024-02-11T20:52:34.000+01:00
* add test project for generic JSX transform

* adapt completion to look up the correct types when using a generic JSX transform

* changelog

* adapt the primitive completions inside of jsx to the generic JSX moe

* remove uneccessary ignore

* update

* update
diff --git a/.gitignore b/.gitignore
@@ -5,7 +5,9 @@ examples/*/lib
 
 analysis/tests/lib
 analysis/tests/.bsb.lock
-analysis/tests/.merlin
+
+analysis/tests-generic-jsx-transform/lib
+analysis/tests-generic-jsx-transform/.bsb.lock
 
 tools/node_modules
 tools/lib
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 - Relax filter for what local files that come up in from and regular string completion in `@module`. https://github.com/rescript-lang/rescript-vscode/pull/918
 - Make from completion trigger for expr hole so we get a nice experience when completing {from: <com>} in `@module`. https://github.com/rescript-lang/rescript-vscode/pull/918
 - Latest parser for newest syntax features. https://github.com/rescript-lang/rescript-vscode/pull/917
+- Handle completion for DOM/element attributes and attribute values properly when using a generic JSX transform. https://github.com/rescript-lang/rescript-vscode/pull/919
 
 ## 1.38.0
 
diff --git a/analysis/Makefile b/analysis/Makefile
@@ -3,16 +3,20 @@ SHELL = /bin/bash
 build-tests:
 	make -C tests build
 
+build-tests-generic-jsx-transform:
+	make -C tests-generic-jsx-transform build
+
 build-reanalyze:
 	make -C reanalyze build
 
-build: build-reanalyze build-tests
+build: build-reanalyze build-tests build-tests-generic-jsx-transform
 
 dce: build-analysis-binary
 	opam exec reanalyze.exe -- -dce-cmt _build -suppress vendor
 
 test-analysis-binary:
 	make -C tests test
+	make -C tests-generic-jsx-transform test
 
 test-reanalyze:
 	make -C reanalyze test
diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml
@@ -1010,14 +1010,23 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
         | Some (builtinNameToComplete, typ)
           when Utils.checkName builtinNameToComplete ~prefix:funNamePrefix
                  ~exact:false ->
+          let name =
+            match package.genericJsxModule with
+            | None -> "React." ^ builtinNameToComplete
+            | Some g ->
+              g ^ "." ^ builtinNameToComplete
+              |> String.split_on_char '.'
+              |> TypeUtils.removeOpensFromCompletionPath ~rawOpens
+                   ~package:full.package
+              |> String.concat "."
+          in
           [
-            Completion.createWithSnippet
-              ~name:("React." ^ builtinNameToComplete)
-              ~kind:(Value typ) ~env ~sortText:"A"
+            Completion.createWithSnippet ~name ~kind:(Value typ) ~env
+              ~sortText:"A"
               ~docstring:
                 [
                   "Turns `" ^ builtinNameToComplete
-                  ^ "` into `React.element` so it can be used inside of JSX.";
+                  ^ "` into a JSX element so it can be used inside of JSX.";
                 ]
               ();
           ]
@@ -1078,7 +1087,7 @@ and getCompletionsForContextPath ~debug ~full ~opens ~rawOpens ~pos ~env ~exact
             | Some f -> Some (f.fname.txt, f.typ, env))
           | _ -> None
         in
-        ["ReactDOM"; "domProps"] |> digToTypeForCompletion
+        TypeUtils.pathToElementProps package |> digToTypeForCompletion
       else
         CompletionJsx.getJsxLabels ~componentPath:pathToComponent
           ~findTypeOfValue ~package
@@ -1692,9 +1701,14 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
     (* We always try to look up completion from the actual domProps type first.
        This works in JSXv4. For JSXv3, we have a backup hardcoded list of dom
        labels we can use for completion. *)
-    let fromDomProps =
+    let pathToElementProps = TypeUtils.pathToElementProps package in
+    if Debug.verbose () then
+      Printf.printf
+        "[completing-lowercase-jsx] Attempting to complete from type at %s\n"
+        (pathToElementProps |> String.concat ".");
+    let fromElementProps =
       match
-        ["ReactDOM"; "domProps"]
+        pathToElementProps
         |> digToRecordFieldsForCompletion ~debug ~package ~opens ~full ~pos ~env
              ~scope
       with
@@ -1713,11 +1727,13 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover completable =
                  else None)
           |> List.map mkLabel)
     in
-    match fromDomProps with
-    | Some domProps -> domProps
+    match fromElementProps with
+    | Some elementProps -> elementProps
     | None ->
       if debug then
-        Printf.printf "Could not find ReactDOM.domProps to complete from.\n";
+        Printf.printf
+          "[completing-lowercase-jsx] could not find element props to complete \
+           from.\n";
       (CompletionJsx.domLabels
       |> List.filter (fun (name, _t) ->
              Utils.startsWith name prefix
diff --git a/analysis/src/CompletionDecorators.ml b/analysis/src/CompletionDecorators.ml
@@ -164,10 +164,17 @@ Example `@raises(Exn)` or `@raises([E1, E2, E3])` for multiple exceptions.
 
 You will need this decorator whenever you want to use a ReScript / React component in ReScript JSX expressions.
 
-Note: The `@react.component` decorator requires the react-jsx config to be set in your `bsconfig.json` to enable the required React transformations.
+Note: The `@react.component` decorator requires the `jsx` config to be set in your `rescript.json`/`bsconfig.json` to enable the required React transformations.
 
 [Read more and see examples in the documentation](https://rescript-lang.org/syntax-lookup#react-component-decorator).|};
       ] );
+    ( "jsx.component",
+      None,
+      [
+        {|The `@jsx.component` decorator is used to annotate functions that are JSX components used with ReScript's [generic JSX transform](https://rescript-lang.org/docs/manual/latest/jsx#generic-jsx-transform-jsx-beyond-react-experimental).
+
+You will need this decorator whenever you want to use a JSX component in ReScript JSX expressions.|};
+      ] );
     ( "return",
       Some "return(${1:nullable})",
       [
diff --git a/analysis/src/CompletionFrontEnd.ml b/analysis/src/CompletionFrontEnd.ml
@@ -659,7 +659,7 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
   let value_binding (iterator : Ast_iterator.iterator)
       (value_binding : Parsetree.value_binding) =
     let oldInJsxContext = !inJsxContext in
-    if Utils.isReactComponent value_binding then inJsxContext := true;
+    if Utils.isJsxComponent value_binding then inJsxContext := true;
     (match value_binding with
     | {pvb_pat = {ppat_desc = Ppat_constraint (_pat, coreType)}; pvb_expr}
       when locHasCursor pvb_expr.pexp_loc -> (
diff --git a/analysis/src/Packages.ml b/analysis/src/Packages.ml
@@ -59,6 +59,16 @@ let newBsPackage ~rootPath =
                | (major, _), None when major >= 11 -> Some true
                | _, ns -> Option.bind ns Json.bool
              in
+             let genericJsxModule =
+               let jsxConfig = config |> Json.get "jsx" in
+               match jsxConfig with
+               | Some jsxConfig -> (
+                 match jsxConfig |> Json.get "module" with
+                 | Some (String m) when String.lowercase_ascii m <> "react" ->
+                   Some m
+                 | _ -> None)
+               | None -> None
+             in
              let uncurried = uncurried = Some true in
              let sourceDirectories =
                FindFiles.getSourceDirectories ~includeDev:true ~baseDir:rootPath
@@ -121,6 +131,7 @@ let newBsPackage ~rootPath =
                ("Opens from ReScript config file: "
                ^ (opens |> List.map pathToString |> String.concat " "));
              {
+               genericJsxModule;
                suffix;
                rescriptVersion;
                rootPath;
diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml
@@ -497,6 +497,7 @@ type builtInCompletionModules = {
 }
 
 type package = {
+  genericJsxModule: string option;
   suffix: string;
   rootPath: filePath;
   projectFiles: FileSet.t;
@@ -718,7 +719,8 @@ module Completable = struct
     | Cpath cp -> "Cpath " ^ contextPathToString cp
     | Cdecorator s -> "Cdecorator(" ^ str s ^ ")"
     | CdecoratorPayload (Module s) -> "CdecoratorPayload(module=" ^ s ^ ")"
-    | CdecoratorPayload (ModuleWithImportAttributes _) -> "CdecoratorPayload(moduleWithImportAttributes)"
+    | CdecoratorPayload (ModuleWithImportAttributes _) ->
+      "CdecoratorPayload(moduleWithImportAttributes)"
     | CdecoratorPayload (JsxConfig _) -> "JsxConfig"
     | CnamedArg (cp, s, sl2) ->
       "CnamedArg("
diff --git a/analysis/src/TypeUtils.ml b/analysis/src/TypeUtils.ml
@@ -1079,3 +1079,8 @@ let removeOpensFromCompletionPath ~rawOpens ~package completionPath =
     |> removeRawOpens rawOpens
   in
   completionPathMinusOpens
+
+let pathToElementProps package =
+  match package.genericJsxModule with
+  | None -> ["ReactDOM"; "domProps"]
+  | Some g -> (g |> String.split_on_char '.') @ ["Elements"; "props"]
diff --git a/analysis/src/Utils.ml b/analysis/src/Utils.ml
@@ -156,10 +156,10 @@ let rec unwrapIfOption (t : Types.type_expr) =
   | Tconstr (Path.Pident {name = "option"}, [unwrappedType], _) -> unwrappedType
   | _ -> t
 
-let isReactComponent (vb : Parsetree.value_binding) =
+let isJsxComponent (vb : Parsetree.value_binding) =
   vb.pvb_attributes
   |> List.exists (function
-       | {Location.txt = "react.component"}, _payload -> true
+       | {Location.txt = "react.component" | "jsx.component"}, _payload -> true
        | _ -> false)
 
 let checkName name ~prefix ~exact =
diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml
@@ -215,9 +215,9 @@ module AddTypeAnnotation = struct
       match si.pstr_desc with
       | Pstr_value (_recFlag, bindings) ->
         let processBinding (vb : Parsetree.value_binding) =
-          (* Can't add a type annotation to a react component, or the compiler crashes *)
-          let isReactComponent = Utils.isReactComponent vb in
-          if not isReactComponent then processPattern vb.pvb_pat;
+          (* Can't add a type annotation to a jsx component, or the compiler crashes *)
+          let isJsxComponent = Utils.isJsxComponent vb in
+          if not isJsxComponent then processPattern vb.pvb_pat;
           processFunction vb.pvb_expr
         in
         bindings |> List.iter (processBinding ~argNum:1);
diff --git a/analysis/tests-generic-jsx-transform/Makefile b/analysis/tests-generic-jsx-transform/Makefile
@@ -0,0 +1,17 @@
+SHELL = /bin/bash
+
+node_modules/.bin/rescript:
+	npm install
+
+build: node_modules/.bin/rescript
+	node_modules/.bin/rescript
+
+test: build
+	./test.sh
+
+clean:
+	rm -r node_modules lib
+
+.DEFAULT_GOAL := test
+
+.PHONY: clean test
diff --git a/analysis/tests-generic-jsx-transform/package-lock.json b/analysis/tests-generic-jsx-transform/package-lock.json
diff --git a/analysis/tests-generic-jsx-transform/package.json b/analysis/tests-generic-jsx-transform/package.json
@@ -0,0 +1,10 @@
+{
+  "scripts": {
+    "build": "rescript",
+    "clean": "rescript clean -with-deps"
+  },
+  "private": true,
+  "dependencies": {
+    "rescript": "11.1.0-rc.2"
+  }
+}
diff --git a/analysis/tests-generic-jsx-transform/rescript.json b/analysis/tests-generic-jsx-transform/rescript.json
@@ -0,0 +1,11 @@
+{
+  "name": "test-generic-jsx-transform",
+  "sources": [
+    {
+      "dir": "src",
+      "subdirs": true
+    }
+  ],
+  "bsc-flags": ["-w -33-44-8"],
+  "jsx": { "module": "GenericJsx" }
+}
diff --git a/analysis/tests-generic-jsx-transform/src/GenericJsx.res b/analysis/tests-generic-jsx-transform/src/GenericJsx.res
@@ -0,0 +1,63 @@
+/* Below is a number of aliases to the common `Jsx` module */
+type element = Jsx.element
+
+type component<'props> = Jsx.component<'props>
+
+type componentLike<'props, 'return> = Jsx.componentLike<'props, 'return>
+
+@module("preact")
+external jsx: (component<'props>, 'props) => element = "jsx"
+
+@module("preact")
+external jsxKeyed: (component<'props>, 'props, ~key: string=?, @ignore unit) => element = "jsx"
+
+@module("preact")
+external jsxs: (component<'props>, 'props) => element = "jsxs"
+
+@module("preact")
+external jsxsKeyed: (component<'props>, 'props, ~key: string=?, @ignore unit) => element = "jsxs"
+
+/* These identity functions and static values below are optional, but lets 
+you move things easily to the `element` type. The only required thing to 
+define though is `array`, which the JSX transform will output. */
+external array: array<element> => element = "%identity"
+@val external null: element = "null"
+
+external float: float => element = "%identity"
+external int: int => element = "%identity"
+external string: string => element = "%identity"
+
+/* These are needed for Fragment (<> </>) support */
+type fragmentProps = {children?: element}
+
+@module("preact") external jsxFragment: component<fragmentProps> = "Fragment"
+
+/* The Elements module is the equivalent to the ReactDOM module in React. This holds things relevant to _lowercase_ JSX elements. */
+module Elements = {
+  /* Here you can control what props lowercase JSX elements should have. 
+  A base that the React JSX transform uses is provided via JsxDOM.domProps, 
+  but you can make this anything. The editor tooling will support 
+  autocompletion etc for your specific type. */
+  type props = {
+    testing?: bool,
+    test2?: string,
+    children?: element
+  }
+
+  @module("preact")
+  external jsx: (string, props) => Jsx.element = "jsx"
+
+  @module("preact")
+  external div: (string, props) => Jsx.element = "jsx"
+
+  @module("preact")
+  external jsxKeyed: (string, props, ~key: string=?, @ignore unit) => Jsx.element = "jsx"
+
+  @module("preact")
+  external jsxs: (string, props) => Jsx.element = "jsxs"
+
+  @module("preact")
+  external jsxsKeyed: (string, props, ~key: string=?, @ignore unit) => Jsx.element = "jsxs"
+
+  external someElement: element => option<element> = "%identity"
+}
diff --git a/analysis/tests-generic-jsx-transform/src/GenericJsxCompletion.res b/analysis/tests-generic-jsx-transform/src/GenericJsxCompletion.res
@@ -0,0 +1,25 @@
+// <div 
+//      ^com
+
+// <div testing={} 
+//               ^com
+
+module SomeComponent = {
+  @jsx.component
+  let make = (~someProp) => {
+	let someString = ""
+    let someInt = 12
+    let someArr = [GenericJsx.null]
+    ignore(someInt)
+    ignore(someArr)
+    // someString->st
+    //               ^com
+	open GenericJsx
+    <div>
+      {GenericJsx.string(someProp)}
+      <div> {GenericJsx.null} </div>
+      // {someString->st}
+      //                ^com
+    </div>
+  }
+}
diff --git a/analysis/tests-generic-jsx-transform/src/expected/GenericJsx.res.txt b/analysis/tests-generic-jsx-transform/src/expected/GenericJsx.res.txt
diff --git a/analysis/tests-generic-jsx-transform/src/expected/GenericJsxCompletion.res.txt b/analysis/tests-generic-jsx-transform/src/expected/GenericJsxCompletion.res.txt
diff --git a/analysis/tests-generic-jsx-transform/test.sh b/analysis/tests-generic-jsx-transform/test.sh
diff --git a/analysis/tests/package-lock.json b/analysis/tests/package-lock.json
diff --git a/analysis/tests/package.json b/analysis/tests/package.json
diff --git a/analysis/tests/src/expected/Completion.res.txt b/analysis/tests/src/expected/Completion.res.txt
diff --git a/analysis/tests/src/expected/CompletionJsx.res.txt b/analysis/tests/src/expected/CompletionJsx.res.txt
