Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exports in ambient module don't get included in certain case #1571

Closed
Unnvaldr opened this issue Apr 15, 2021 · 4 comments · Fixed by #1576
Closed

Exports in ambient module don't get included in certain case #1571

Unnvaldr opened this issue Apr 15, 2021 · 4 comments · Fixed by #1576
Labels
bug Functionality does not match expectation

Comments

@Unnvaldr
Copy link
Contributor

Search terms

ambient, module, export, exports, import, imports, not, included

Expected Behavior

Shared types imported from other ambient module should be included in both ambient modules.

Actual Behavior

Shared types imported from other ambient module only get included in 1st ambient module.

(Types that are missing from alt-server are marked with red dot on the right)
(Note that also functions are missing, but they're not visible here)

Excerpt

Steps to reproduce the bug

Clone this repo with specified branch and generate documentation by executing ./docs/build.ps1 script (only Windows is supported for now):
https://github.com/altmp/altv-types/tree/shared-package
You should be able to check documentation on localhost:8080 or simply check ./docs/api/.manifest, which contains renamed typedoc JSON output. (In case of problems, simply run npm i and npx typedoc inside ./docs/ directory)

Environment

  • Typedoc version: 0.20.35
  • TypeScript version: 4.2.4
  • Node.js version: 14.15.1
  • OS: Windows 10 Pro v1809 x64
@Unnvaldr Unnvaldr added the bug Functionality does not match expectation label Apr 15, 2021
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 23, 2021

There are a couple issues here:

  1. Paths in TypeDoc's config file aren't relative to the config file (Paths in option files are resolved relative to cwd #1442), so your config file doesn't work, unless you somehow have a npm repo inside the docs folder... which seems unlikely. (npx always executes from the project root)
  2. generateJson doesn't properly create folders for the file to be written (not what you were reporting, but something I noticed while testing)

I changed your config file as shown:

diff --git a/docs/typedoc.json b/docs/typedoc.json
index a2f3c5e..a244462 100644
--- a/docs/typedoc.json
+++ b/docs/typedoc.json
@@ -1,8 +1,8 @@
 {
   "entryPoints": [
-    "../client",
-    "../server"
+    "client",
+    "server"
   ],
   "excludeExternals": true,
-  "json": "api/.manifest"
+  "json": "docs/api/.manifest"
 }

And then ran npx typedoc --options docs/typedoc.json --out html_docs.

This produced the expected output. In alt-client, Vector2 was documented as a class. In alt-server, TypeDoc saw that it has already documented the class, and instead of duplicating the work, produced an alias pointing to the class in the alt-client documentation.

If aliases aren't showing up in your DocFX docs, then there's a problem with that tool... the HTML rendered from TypeDoc looks as I expect.

Gerrit0 added a commit that referenced this issue Apr 23, 2021
@Unnvaldr
Copy link
Contributor Author

I've missed at some point that paths aren't relative to the config file;
Good catch with JSON output path, for me path is already created at the time TypeDoc is running, so didn't have a chance 😄

Although alias thing seems to be proper when there'd be additional ambient module exporting these types and then other ambient modules re-exporting them, which sadly is not the case here. While making alias to first ambient module is smart from technical point, it's not valid from my logical point of view. 1st & 2nd ambient module have no connection to each other, so they should've exported these types separately anyway.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 23, 2021

With #1567, I believe you should be able to achieve what you want by including each module as packages rather than entry points.

Using aliases makes sense for most projects.. I'm not fundamentally opposed to introducing a --disableAliases option which turns that off though.

@Unnvaldr
Copy link
Contributor Author

Unnvaldr commented Apr 23, 2021

With #1567, I believe you should be able to achieve what you want by including each module as packages rather than entry points.

Using aliases makes sense for most projects.. I'm not fundamentally opposed to introducing a --disableAliases option which turns that off though.

Yes, I'm already implementing a* switch for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality does not match expectation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants