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

Migrate JSDocs to TSDocs #2756

Merged
merged 30 commits into from
Jul 8, 2023
Merged

Migrate JSDocs to TSDocs #2756

merged 30 commits into from
Jul 8, 2023

Conversation

HarelM
Copy link
Member

@HarelM HarelM commented Jun 28, 2023

Resolves #2150
Resolves #2776
This will be a PR that will be merged when the branch is ready and all comments are migrated.
Please open PRs against this PR/branch.

  • CI - will be solved by
  • Enable TSdocs lint? (mainly @param hyphen?)
  • Make sure the docs don't have warning when they build
  • remove @memberof @instanceof @inherits, @typedef, @name, @function, @instance, @property
  • Event are linking to invalid location and in general are not properly structured in terms of docs, I'll need to see how to properly solve this. Not a blocker
  • Make sure example links are working and are backward compatible (?)
  • Update the main readme with the relevant links

HarelM and others added 16 commits July 4, 2023 10:40
* Fix some controls

* Replace jsdoc with tsdocs lint

* Fix control's jsdocs

* Improve the looks of the controls

* Fix lint and CI

* Remove the last jsdoc comments in geolocation control
…2674)

* rendering test: implement cropping of png result

This is useful for renderer results that are too large to be handled by puppeteer

* mock webgl: update drawing buffer size when viewport changes

* Round down painter and canvas dimensions so they're consistent

* add veryhigh pixel ratio test

This test draws a black square,
The pixel ratio and width were chosen to exceed the maximum texture value, which is set to 8192 in the CI environment.
The result is cropped due to the limits of the intercommunication between chromium and puppeteer.

* introduce maxCanvasSize option

A canvas size over GL MAX_TEXTURE_SIZE can cause distortions and an excessive canvas size can cause hangs or crashes.
Limiting canvas size can fix these issues. Add an option to let the users alter this limit.
The default value is [4096, 4096] as MAX_TEXTURE_SIZE is usally at least 4096px.
In CI max_texture_size is 8192, so we set maxCanvasSize to that value in tests.

* Resize map after setting pixel ratio

This has two benefits:
1. The map is immediately re-rendered with the new pixel ratio
2. We avoid duplicating pixel ratio clamping logic

* further clamp pixel ratio after hitting gl limits

Althoug setting maxCanvasSize to a value <= GL.MAX_TEXTURE_SIZE solves most problems, there is no guarantee that we cannot exceed other GL limits.
To handle these cases, we check that the drawing buffer is of the requested size, if it isn't we scale down pixel ratio further

* map test: move webgl error in its own section

* add a unit test for webgl drawing buffer limits

* update maxCanvasSize type and change comment type

* update maxCanvasSize after hitting gl limits

This is an optimization to avoid failing the overlimit check multiple times.

* update changelog
* Added handlers to the docs

* Fix lint
* Changes to docs contribution

* don't link for better review
* Migrated marker and popup

* Fix lint

* Code review fixes

* Added missing marker options object

* Fix lint and tests

* Fix typo

* remove unneeded lint comment.
Bumps [typescript](https://github.com/Microsoft/TypeScript) from 5.1.5 to 5.1.6.
- [Release notes](https://github.com/Microsoft/TypeScript/releases)
- [Commits](https://github.com/Microsoft/TypeScript/commits)

---
updated-dependencies:
- dependency-name: typescript
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix sources

* Added typedoc

* Fix custom style layer
* insets, lnglat+bounds

* mercator

* transform

* draw_terrain

* line_atlas

* terrain

* tile, styel_image

* actore

* ajax

* image request queue

* struct array

* Remove returns objects

* typedef

* remove function

* Removed some prperties

* Fix lint

* Fix tests

* Lint, cr fixes
@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2023

Codecov Report

Patch coverage: 89.21% and project coverage change: +0.03 🎉

Comparison is base (30c0e1d) 73.90% compared to head (8946e89) 73.93%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2756      +/-   ##
==========================================
+ Coverage   73.90%   73.93%   +0.03%     
==========================================
  Files         238      238              
  Lines       18980    18968      -12     
  Branches     4282     4281       -1     
==========================================
- Hits        14027    14024       -3     
+ Misses       4953     4944       -9     
Impacted Files Coverage Δ
src/data/bucket.ts 92.85% <ø> (ø)
src/data/bucket/circle_bucket.ts 78.16% <ø> (ø)
src/data/bucket/line_bucket.ts 58.23% <ø> (ø)
src/data/bucket/symbol_bucket.ts 50.61% <ø> (ø)
src/data/evaluation_feature.ts 100.00% <ø> (ø)
src/data/extent.ts 100.00% <ø> (ø)
src/data/feature_index.ts 32.88% <ø> (ø)
src/data/index_array_type.ts 100.00% <ø> (ø)
src/data/load_geometry.ts 100.00% <ø> (ø)
src/data/program_configuration.ts 32.28% <ø> (ø)
... and 110 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@HarelM HarelM marked this pull request as ready for review July 5, 2023 10:31
@HarelM HarelM requested a review from birkskyum July 5, 2023 10:32
@HarelM
Copy link
Member Author

HarelM commented Jul 5, 2023

This is ready for review, basically most if not all of the content here was reviewed in the PRs that where merged to this branch.

@birkskyum
Copy link
Member

Should #2802 be merged in first?

* Added comments to reduce warnings

* Fix most of the warnings in the build process
@HarelM
Copy link
Member Author

HarelM commented Jul 5, 2023

Yes, and also the CI part, as part of the #2776 bounty.
But all the PRs are against this branch, if this is not approved I won't be able to merge.
If you want to re-review everything again that fine too 😀

@birkskyum
Copy link
Member

Would you do the @param hyphen replacement as part of this?

@HarelM
Copy link
Member Author

HarelM commented Jul 5, 2023

IDK, I might as part of enabling the tsdocs lint stuff..

@HarelM
Copy link
Member Author

HarelM commented Jul 5, 2023

I've finished with most of the tsdoc errors.
I'm giving the events a final touch.
Hopefully a new doc site by the end of the week.

* Move all examples to be tripple slash, hypen, remove @Private

* delimiter fix

* More fixes

* Fix lint

* Remove unneeded rule

* Code review changes.

* Fix var let bad replacenment

* Fix typo

* Fix bwc address
@HarelM
Copy link
Member Author

HarelM commented Jul 5, 2023

I'm struggling with what appears to be a bug with the markdown plugin:
typedoc2md/typedoc-plugin-markdown#448

I'll leave this for now as I would like to have more groups in the main intro file and currently something doesn't like me there...

@HarelM
Copy link
Member Author

HarelM commented Jul 6, 2023

The following might also be a solution to the above issue if I don't get any feedback on the issue I opened:
https://github.com/hughrawlinson/typedoc-to-markdown
It's a single file converting the typedoc json output to markdown, which seems a lot simpler.

@birkskyum
Copy link
Member

I only see these items in the toc - should I expect to see also the classes/interface/enums?

Screenshot 2023-07-06 at 10 51 05

@birkskyum
Copy link
Member

If it's appear to be completely impossible to repair the markdown/mkdocs flow, there is also a potential option to explore if any of the other typedoc plugins work (docusaurus or even hugo) to figure if it's the tsdocs output that's off. Don't know how much adaption of examples that would be though.

@HarelM
Copy link
Member Author

HarelM commented Jul 6, 2023

The problem is with the markdown plugin, as you can see in the linked issue.
I've checked the typedoc json output and it is valid and with the relevant group.
Docusurus also uses this ("buggy") plugin, so we'll be facing the same issue.
The problem is with the markdown generation, other platforms won't be able to help with this I believe...

@HarelM
Copy link
Member Author

HarelM commented Jul 6, 2023

Since converting the json file to markdown files shouldn't be complicated, and there is already some code that does that, it might be a possible solution. But's let's wait a bit to see if there's a response.

@HarelM
Copy link
Member Author

HarelM commented Jul 6, 2023

Event Related was now added by adding a config flag to place everything in a different file.
There is still a need to fix the MapEvent docs as they are scrambled due to having @see and @exmaple one after the other.
And also improve the docs above the on to link to the relevant places.
I still need to think about how to improve this.
Also the main readme is not pointing relatively to the docs which I need to fix as well.
These are minor fixes, the last part of all this is the CI.
image

@HarelM
Copy link
Member Author

HarelM commented Jul 6, 2023

With current version the events are now defined in two tables with the MapLayerEventType and MapEventType.
I have removed the links in the Map#on method as they didn't point anywhere and added above that table the relevant link.
I'm good with how it turned out.
Last thing is the main readme and the CI.
Overall this is almost complete.

@birkskyum
Copy link
Member

birkskyum commented Jul 8, 2023

All the links inside of the module, like:
https://maplibre.org/maplibre-gl-js/docs/API/classes/maplibregl.AttributionControl/

Was before found at:
https://maplibre.org/maplibre-gl-js-docs/api/markers/#attributioncontrol

Is there a migration strategy for that?

@HarelM
Copy link
Member Author

HarelM commented Jul 8, 2023

The main links that I believe we will be able to keep are the examples bybredirecting maplibre-gl-js-docs to maplibre-gl-js/docs
I don't think I have a good idea how to keep the old link to classes etc, especially since it's case sensitive and the old links are lowercase...
Having said that, I'm OK with it, classes can change name etc and links to classes my break over time.

* Fix main readme links

* make links relative

* More readme fixes

* Adding build to CI

* Simplify docker commend

* Change site url in the config to match the expected production site.

* Added CI part
@HarelM
Copy link
Member Author

HarelM commented Jul 8, 2023

I'll merge this and create a pre-release to see that everything is working asv expected.
Once everything is working I'll update the condition not to publish the docs on pre-release.

@birkskyum
Copy link
Member

Sounds good. I did btw. try the @next version of the markdown plugin, but things are breaking, and it's also behind the master branch with 60+ commits, so it's probably best to proceed with the current version.

@HarelM HarelM merged commit f329d91 into main Jul 8, 2023
14 checks passed
@HarelM HarelM deleted the jsdocs-tsdocs branch July 8, 2023 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring in the docs repo - CI Bring in the docs repo
5 participants