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

Use @namespace in addition to @enum for jsdoc #10259

Open
IanLilleyT opened this issue Apr 1, 2022 · 3 comments
Open

Use @namespace in addition to @enum for jsdoc #10259

IanLilleyT opened this issue Apr 1, 2022 · 3 comments

Comments

@IanLilleyT
Copy link
Contributor

IanLilleyT commented Apr 1, 2022

Old

When using the @enum tag by itself it puts the documentation in the global section and omits static functions on the enum. It's cluttered and missing information.

Screenshot from 2022-04-01 10-49-15

New

By adding the @namespace tag it creates a new page for the enum and show the static functions:
Screenshot from 2022-04-01 10-49-37


Here's how the fix would look for VoxelShapeType:

/**
 * An enum of voxel shapes supported by <code>EXT_primitive_voxels</code>. The shape controls
 * how the voxel grid is mapped to 3D space.
 *
 * @namespace
 * @enum VoxelShapeType
 */

One way to go about fixing this for all enums is to:

  • Run npm run generateDocumentation
  • Go to http://localhost:8080/Build/Documentation/global.html
  • Add @namespace to every enum until there are no more in the global section

Also, update https://github.com/CesiumGS/cesium/tree/main/Documentation/Contributors/DocumentationGuide with information about enums / namespace.

@IanLilleyT IanLilleyT added cleanup category - doc good first issue An opportunity for first time contributors labels Apr 1, 2022
@mramato
Copy link
Contributor

mramato commented Apr 1, 2022

We need to be careful here. For things in the public API, making them a namespace means they are not enums, they are namespaces. This matters a lot when generating TypeScript bindings. However, if we are globbing functions on to enums, they aren't enums either.

I did a bunch of work to make sure we made enums actually enums as part of the initial TS works: #8878 This included removing functions being globbed onto them and changing them to actually use @enum in JSDoc.

CesiumJS should have a clear policy for all of this, what we have may be out of date.

If the goal is to put them into their own page, we could probably do that at the JSDoc template level.

What does this do to TS output?

@IanLilleyT
Copy link
Contributor Author

Ahh good catch. When I do npm run build-ts it errors with:

Source/Cesium.d.ts:28217:21 - error TS2709: Cannot use namespace 'VoxelShapeType' as a type.
28217     readonly shape: VoxelShapeType;

The goals are:

  • Preserve the documentation for static functions (most important)
  • Put enums and their static functions into their own page (JSDoc template idea) (less important but still good to have)

Are there any examples of well behaved public enums that have static functions? Any workarounds you found during the TS cleanup? It seems like an increasingly common pattern in the codebase. For example, it's nice to be able to do VoxelShapeType.BOX and VoxelShapeType.getMinBounds(shape).

EasingFunction is the closest I can find. It's an enum that uses @namespace. TweenCollection references it with @type {EasingFunction} but since TweenCollection is @private the TS build doesn't catch the mistake.

@mramato
Copy link
Contributor

mramato commented Apr 2, 2022

The fundamental issue here is that an enums don't exist in JavaScript (because types don't exist). I don't know of any strongly typed language that lets you define an enum that also has methods on it (static or not). So we should not every augment enums with function definitions in Cesium is we want to cleanly interop with strongly typed languages, like TypeScript.

I believe the documentation side can be handled entirely in JSDoc. I don't know the direct solution off the top of my head, but JSDoc gives you a lot of control over how the doc is generated.

@ggetz ggetz removed the good first issue An opportunity for first time contributors label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants