add `moduletypeid` field for modules with explicitly annotated module type #1019

aspeddro · 2024-06-10T13:56:51Z

No description provided.

annotated module type

woeps

Thanks for your efforts!
Please, see other comments.

woeps · 2024-06-10T16:08:56Z

tools/src/tools.ml

@@ -44,7 +44,14 @@ type docItem =
          (** Additional documentation for constructors and record fields, if available. *)
    }
  | Module of docsForModule
-  | ModuleType of docsForModule
+  | ModuleType of {
+      id: string;


Did you update the rescript types accordingly?

woeps · 2024-06-10T16:14:30Z

tools/src/tools.ml

+                         extractDocsForModule ~modulePath:(m.name :: modulePath)
+                           m
+                       in
+                       Some (Module {docs with moduletype = Some (Path.name p)})


Is this the same string, which is used for the id of the module type, when generating json for the module type?

For further processing it would make sense, to use the id. If so, the record field should probably renamed as well (moduleTypeId).

Renamed field to moduletypeid bc all fields are in lowercase.

a67efa5 added full id DocExtractionRes.Example

aspeddro · 2024-06-11T02:24:41Z

tools/tests/src/expected/DocExtractionRes.res.json

@@ -311,6 +311,7 @@
    "id": "DocExtractionRes.M",
    "name": "M",
    "kind": "module",
+    "moduletypeid": "DocExtractionRes.Example",


@woeps, same id of module type

zth · 2024-06-11T06:41:51Z

Perhaps it'd be better to have both the module type name and the module type id?

woeps · 2024-06-11T06:44:59Z

Looks good to me! :D

The only possible issue I found is, when the explicitly annotated module type is in another file:

// Types.res
module type T = {
  let x: int
}

// Example.res
module M: Types.T = {
  let x = 42
}

Above will result in moduletypeid=Example.Types.T in the json generated for module M.
I would expect the id to be Types.T.

PS: module types in the same file are working as expected.

woeps · 2024-06-11T16:34:36Z

Perhaps it'd be better to have both the module type name and the module type id?

Tbh, I don't think the name is necessary in the light of progrmatically processing the data to e.g. build a docs website.
In the sense of a "normalized dataset", I just need the reference on where to find the details.

But if you (@zth) have a usecase in mind, where the name would be convenient to have without further id resolutions, I'm not opposed.
I'm just saying I dont't necessary need the name for the usecases I'm thinking about.

zth · 2024-06-12T06:44:38Z

No I don't have anything specific in mind really. Just a thought.

Is this ready to go?

aspeddro · 2024-06-12T14:24:52Z

No, I'll take a look later

aspeddro · 2024-06-12T23:21:14Z

Right, moduletypeid fixed.

Example.res

module type T = {
  let a: int
}
module M: Types.T = {
  let x = 42
}
module A: T = {
  let a = 1
}

Types.res

module type T = {
  let x: int
}

{
  "name": "Example",
  "docstrings": [],
  "source": {
    "filepath": "src/Example.res",
    "line": 1,
    "col": 1
  },
  "items": [
  {
    "id": "Example.T",
    "name": "T",
    "kind": "moduleType",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 13
    },
    "items": [
    {
      "id": "Example.T.a",
      "kind": "value",
      "name": "a",
      "signature": "let a: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 2,
        "col": 3
      }
    }]
  }, 
  {
    "id": "Example.M",
    "name": "M",
    "kind": "module",
    "moduletypeid": "Types.T",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.M.x",
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 5,
        "col": 7
      }
    }]
  }, 
  {
    "id": "Example.A",
    "name": "A",
    "kind": "module",
    "moduletypeid": "T",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.A.a",
      "kind": "value",
      "name": "a",
      "signature": "let a: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 8,
        "col": 7
      }
    }]
  }]
}

aspeddro · 2024-06-12T23:24:43Z

Ready to go!

woeps · 2024-06-13T04:55:40Z

Right, moduletypeid fixed.

Example.res

module type T = {
  let a: int
}
module M: Types.T = {
  let x = 42
}
module A: T = {
  let a = 1
}

Types.res

module type T = {
  let x: int
}

Sorry to be a downer 🫣:
I believe the generated moduletype for module A should be Example.T. (in the given Example the filename Example is missing)

I'll check if this is just a typo in the comments or the actual implementation in a bit.

woeps · 2024-06-13T07:31:19Z

I just verified: the json example in the comment above from @aspeddro is according to the implementation.

To me, it seems it is a little bit more complicated than just passing modulepath to makeId or not.

My expectation of the generated moduletypeid when a module type is explictly annotated inside a file Example.resi: <FileName>.<PathInFile>.<ModuleTypeName>

zth · 2024-06-14T07:35:23Z

@woeps I interpret this as the PR isn't ready to go just yet?

woeps · 2024-06-14T10:55:28Z

@woeps I interpret this as the PR isn't ready to go just yet?

@zth as I see it, no. This isn't ready yet.

aspeddro · 2024-06-15T21:57:27Z

Ok, I think is this, right?

{
  "name": "Example",
  "docstrings": [],
  "source": {
    "filepath": "src/Example.res",
    "line": 1,
    "col": 1
  },
  "items": [
  {
    "id": "Example.T",
    "name": "T",
    "kind": "moduleType",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 13
    },
    "items": [
    {
      "id": "Example.T.a",
      "kind": "value",
      "name": "a",
      "signature": "let a: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 2,
        "col": 3
      }
    }]
  }, 
  {
    "id": "Example.M",
    "name": "M",
    "kind": "module",
    "moduletypeid": "Types.T",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.M.x",
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 6,
        "col": 7
      }
    }]
  }, 
  {
    "id": "Example.A",
    "name": "A",
    "kind": "module",
    "moduletypeid": "Example.T",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.A.a",
      "kind": "value",
      "name": "a",
      "signature": "let a: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 10,
        "col": 7
      }
    }]
  }]
}

woeps · 2024-06-16T18:40:32Z

@aspeddro great progress! 👍

I found one more case, which needs to be handled (when submodules have explicit module types):

// Example.res
module type MT = {
  let x: int
}

module A: MT = {
  let x = 42
}

module B: Types.T = {
  let x = 42
}

module C = {
  module D: MT = // generates "moduletypeid": "Example.C.MT" - but should be Example.MT
    let x = 42
  }
  module E: Types.T = {
    let x = 3
  }
}

generates this json:

{
  "name": "Example",
  "docstrings": [],
  "source": {
    "filepath": "test/Example.res",
    "line": 1,
    "col": 1
  },
  "items": [
  {
    "id": "Example.MT",
    "name": "MT",
    "kind": "moduleType",
    "docstrings": [],
    "source": {
      "filepath": "test/Example.res",
      "line": 1,
      "col": 13
    },
    "items": [
    {
      "id": "Example.MT.x",
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "test/Example.res",
        "line": 2,
        "col": 3
      }
    }]
  }, 
  {
    "id": "Example.A",
    "name": "A",
    "kind": "module",
    "moduletypeid": "Example.MT",
    "docstrings": [],
    "source": {
      "filepath": "test/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.A.x",
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "test/Example.res",
        "line": 6,
        "col": 7
      }
    }]
  }, 
  {
    "id": "Example.B",
    "name": "B",
    "kind": "module",
    "moduletypeid": "Types.T",
    "docstrings": [],
    "source": {
      "filepath": "test/Example.res",
      "line": 1,
      "col": 1
    },
    "items": [
    {
      "id": "Example.B.x",
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "test/Example.res",
        "line": 10,
        "col": 7
      }
    }]
  }, 
  {
    "id": "Example.C",
    "name": "C",
    "kind": "module",
    "docstrings": [],
    "source": {
      "filepath": "test/Example.res",
      "line": 13,
      "col": 8
    },
    "items": [
    {
      "id": "Example.C.D",
      "name": "D",
      "kind": "module",
      "moduletypeid": "Example.C.MT",  <-- this should be Example.MT
      "docstrings": [],
      "source": {
        "filepath": "test/Example.res",
        "line": 1,
        "col": 1
      },
      "items": [
      {
        "id": "Example.C.D.x",
        "kind": "value",
        "name": "x",
        "signature": "let x: int",
        "docstrings": [],
        "source": {
          "filepath": "test/Example.res",
          "line": 15,
          "col": 9
        }
      }]
    }, 
    {
      "id": "Example.C.E",
      "name": "E",
      "kind": "module",
      "moduletypeid": "Types.T",
      "docstrings": [],
      "source": {
        "filepath": "test/Example.res",
        "line": 1,
        "col": 1
      },
      "items": [
      {
        "id": "Example.C.E.x",
        "kind": "value",
        "name": "x",
        "signature": "let x: int",
        "docstrings": [],
        "source": {
          "filepath": "test/Example.res",
          "line": 18,
          "col": 9
        }
      }]
    }]
  }]
}

on a side note: It seems source for module A, B, D & E is incorrect: I'll create a new issue, once this pr is merged.

zth · 2024-06-16T18:48:53Z

Thanks for both of your work! Keep it going! 😄

aspeddro · 2024-06-16T20:58:22Z

Added some tests a40df41

woeps · 2024-06-16T21:24:26Z

@zth I think this is now good to be merged.

zth · 2024-06-17T05:12:17Z

Great work! Do you want me to put out another tools version with this or is there other outstanding things we should try and fix before?

woeps · 2024-06-17T05:37:34Z

@zth You may publish a new tools version.
This PR stands on its own and is a great step forward. 🥳

zth · 2024-06-17T06:19:40Z

@woeps 0.6.4 is on its way out.

… type (rescript-lang#1019) * add `moduletype` field for modules with explicitly annotated module type * add full id * remove modulePath * update CHANGELOG.md * fix moduletypeid * ignore parent module name * add tests * remove comment

add moduletype field for modules with explicitly

0114650

annotated module type

aspeddro mentioned this pull request Jun 10, 2024

docgen should name module type for explicitly annotated module type #1010

Closed

woeps reviewed Jun 10, 2024

View reviewed changes

add full id

a67efa5

aspeddro commented Jun 11, 2024

View reviewed changes

aspeddro added 2 commits June 12, 2024 20:21

remove modulePath

162899d

update CHANGELOG.md

14aba6a

aspeddro changed the title ~~add moduletype field for modules with explicitly annotated module type~~ add moduletypeid field for modules with explicitly annotated module type Jun 12, 2024

fix moduletypeid

4ef0bde

aspeddro added 2 commits June 16, 2024 17:52

ignore parent module name

de20676

add tests

a40df41

remove comment

862eff9

zth merged commit 50bc12f into rescript-lang:master Jun 17, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `moduletypeid` field for modules with explicitly annotated module type #1019

add `moduletypeid` field for modules with explicitly annotated module type #1019

aspeddro commented Jun 10, 2024

woeps left a comment

woeps Jun 10, 2024

aspeddro Jun 11, 2024

woeps Jun 10, 2024

aspeddro Jun 11, 2024

aspeddro Jun 11, 2024

zth commented Jun 11, 2024

woeps commented Jun 11, 2024 •

edited

Loading

woeps commented Jun 11, 2024

zth commented Jun 12, 2024

aspeddro commented Jun 12, 2024

aspeddro commented Jun 12, 2024

aspeddro commented Jun 12, 2024

woeps commented Jun 13, 2024 •

edited

Loading

woeps commented Jun 13, 2024

zth commented Jun 14, 2024

woeps commented Jun 14, 2024

aspeddro commented Jun 15, 2024

woeps commented Jun 16, 2024

zth commented Jun 16, 2024

aspeddro commented Jun 16, 2024

woeps commented Jun 16, 2024

zth commented Jun 17, 2024

woeps commented Jun 17, 2024

zth commented Jun 17, 2024

add moduletypeid field for modules with explicitly annotated module type #1019

add moduletypeid field for modules with explicitly annotated module type #1019

Conversation

aspeddro commented Jun 10, 2024

woeps left a comment

Choose a reason for hiding this comment

woeps Jun 10, 2024

Choose a reason for hiding this comment

aspeddro Jun 11, 2024

Choose a reason for hiding this comment

woeps Jun 10, 2024

Choose a reason for hiding this comment

aspeddro Jun 11, 2024

Choose a reason for hiding this comment

aspeddro Jun 11, 2024

Choose a reason for hiding this comment

zth commented Jun 11, 2024

woeps commented Jun 11, 2024 • edited Loading

woeps commented Jun 11, 2024

zth commented Jun 12, 2024

aspeddro commented Jun 12, 2024

aspeddro commented Jun 12, 2024

aspeddro commented Jun 12, 2024

woeps commented Jun 13, 2024 • edited Loading

woeps commented Jun 13, 2024

zth commented Jun 14, 2024

woeps commented Jun 14, 2024

aspeddro commented Jun 15, 2024

woeps commented Jun 16, 2024

zth commented Jun 16, 2024

aspeddro commented Jun 16, 2024

woeps commented Jun 16, 2024

zth commented Jun 17, 2024

woeps commented Jun 17, 2024

zth commented Jun 17, 2024

add `moduletypeid` field for modules with explicitly annotated module type #1019

add `moduletypeid` field for modules with explicitly annotated module type #1019

woeps commented Jun 11, 2024 •

edited

Loading

woeps commented Jun 13, 2024 •

edited

Loading