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

Fix SphinxJS/TypeDoc integration #1199

Merged
merged 14 commits into from
Oct 6, 2020
Merged

Fix SphinxJS/TypeDoc integration #1199

merged 14 commits into from
Oct 6, 2020

Conversation

raucao
Copy link
Member

@raucao raucao commented Sep 22, 2020

The new version of SphinxJS has been released, and it has official TypeDoc support now. I successfully got it running on my machine. But I had to refactor the last remaining non-native classes/functions for the autofunction lookups to work.

closes #1192

@raucao raucao marked this pull request as ready for review September 23, 2020 10:27
@raucao raucao requested a review from a team September 23, 2020 10:28
@raucao
Copy link
Member Author

raucao commented Sep 23, 2020

Something is still wrong with ReadTheDocs' TypeScript/TypeDoc setup: https://readthedocs.org/projects/remotestoragejs/builds/11943970/

It's missing our npm type dependencies apparently, and it looks like it's also using outdated TS. I'm having a look.

@raucao
Copy link
Member Author

raucao commented Sep 23, 2020

I couldn't make it work so far. Looking at a raw build log, it seems to me like RTD is not actually running npm i during the build process, so we'd have to check in and push the node_modules folder.

Improved some other things on the way, like e.g. having more than just master/latest being published. v1.2.3 is now published explicitly in addition, and used as default redirect. Visitors can then switch to "latest" if they choose so.

Edit: I found that they actually have a support team and contact, so I wrote them an email about our problem.

@raucao
Copy link
Member Author

raucao commented Sep 24, 2020

Eric from RTD got back to me and suggested to just hackfix the pre-build step by shelling out from conf.py, which is a Python file that is being executed before the build. So I tried that, and it worked immediately:

https://remotestoragejs--1199.org.readthedocs.build/en/1199/contributing/docs.html

Everything ready to merge now, and it will publish the new docs as "latest", as soon as this PR lands in master. (New default then still being "v1.2.3"/stable.)

It's almost the same as 'autobuild-docs', but the short version of the
ignore arguments (-i) isn't supported anymore by sphinx-autobuild.
@galfert
Copy link
Member

galfert commented Sep 24, 2020

Documentation builds are working for me.

For the autobuilding I first tried running npm run doc, because that's the first task I found in the package.json.

But that task faild with an error:

sphinx-autobuild: error: unrecognized arguments: -i -i -i #*# . _build/html

Looks like the shorthand version for the --ignore argument isn't supported anymore by sphinx-autobuild.

But then I noticed that the documentation documentation (doc/contributing/docs.rst) mentions the task autobuild-docs instead. It's the same as the doc task, just without the ignore arguments. So I removed the doc task to prevent any further confusion.

In the doc builds themselves I noticed that all number arguments have the type unknown now:

Screen Shot 2020-09-24 at 14 04 43

But I guess that can be fixed as part of #1201.

@raucao
Copy link
Member Author

raucao commented Sep 24, 2020

In the doc builds themselves I noticed that all number arguments have the type unknown now:

We'll have to find a solution for that, because the type is actually supposed to be "unknown" in the code, so that TypeScript forces us to validate the unknown input from app authors/code. But yeah, I agree that we can handle that later.

@raucao
Copy link
Member Author

raucao commented Sep 25, 2020

I couldn't find a solution for the unknown type problem, so I created an issue about it here: TypeStrong/typedoc#1370

@raucao
Copy link
Member Author

raucao commented Oct 5, 2020

Talked about it with @galfert and we came to the conclusion that using normal types instead of unknown is the best solution here. Losing the type safety isn't worth the effort in this case.

@raucao
Copy link
Member Author

raucao commented Oct 5, 2020

I just pushed some commits to address the last documentation issue (unknown argument types). Ready to merge AFAICS.

/cc @galfert

@galfert
Copy link
Member

galfert commented Oct 6, 2020

Looks good to me 👍

@raucao raucao merged commit bda6bf7 into master Oct 6, 2020
@raucao raucao deleted the feature/sphinx_typedoc branch October 6, 2020 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix sphinx-js integration
2 participants