mozilla / sphinx-js

Autodoc-style extraction into Sphinx for your JS project
https://pypi.python.org/pypi/sphinx-js/
MIT License
282 stars 81 forks source link

extend typename of typeParameter with constraint #124

Closed bollwyvl closed 4 years ago

bollwyvl commented 5 years ago

Dupe of #119, probably: feel free to close!

Here's a fun one... yields:

# Sphinx version: 2.2.1
# Python version: 3.7.3 (CPython)
# Docutils version: 0.15.2 release
# Jinja2 version: 2.10.3

# typedoc: 0.15.2  # added by me

# Last messages:

Exception occurred:
  File ".../sphinx_js/typedoc.py", line 152, in <listcomp>
    argNames = ['|'.join(self.make_type_name(arg)) for arg in type.get('typeArguments')]
TypeError: sequence item 2: expected str instance, list found

(nothing much else in the log)

it appears that make_type_name is always expected to return a List[Text]: this change ensures a typeParameter with a constraint does not become List[Union[Text, List[Text]]... or worse! I see some of them are having the 0th item taken, would that be more appropriate?

Happy to work up a test... i see there are some ts files that are getting round-tripped, which includes a pretty gnarly generic, as well as a build example... what would be most beneficial to add?

However, I can't claim to have gotten all the way down the rabbit hole due to our use of lerna, and trying to make typedoc-plugin-lerna-packages work as well... my environment is kind of a mess of hacks right now.

erikrose commented 4 years ago

Thank you for the patch! I don't use TypeScript myself, so I have little idea what's going on in our TS support and am thus a poor judge. So I would put it to you: how do you think your fix compares with graup's fix, linked from https://github.com/mozilla/sphinx-js/issues/119#issuecomment-537357793? I am inclined to wait for a test to merge, as well, especially in the absence of better understanding of the code on my part. :-)

erikrose commented 4 years ago

This is fixed in the great IR merge of #120, demonstrated by https://github.com/mozilla/sphinx-js/blob/705659ceb08de1ec1fd16bdefd53e7b2e0a8d848/tests/test_typedoc_analysis/test_typedoc_analysis.py#L375-L379.

bollwyvl commented 4 years ago

Amazing stuff! Excited to take a look with the jupyterlab and lumino code bases!

erikrose commented 4 years ago

Let me know how it goes! I'm interested to get some real-world proof that it works before releasing it; there were a lot of changes!