Add initial conversion script from Documentation.js to old format

[davepagurek]

This adds some boilerplate for a node script to convert docs/data.json to the old docs/reference/data.json format. Run it by calling node utils/convert.js and it'll dump the output in docs/converted.json.

• [x] Handle examples
• [x] Handle descriptions
• [x] Convert basic methods
• [x] Handle multiple method overloads
• [x] Handle module/submodule categorization
• [x] Handle properties
• [x] Handle class listings
• [x] Handle constants
• [x] Update other p5 ES6 classes to follow best practices for properties and constructors

[davepagurek]

@limzykenneth I hit a bit of a snag with detecting methods in ES6 classes correctly. It seems like in some cases like in p5.Framebuffer, documentation.js isn't correctly labeling its methods as being a memberof p5.Framebuffer. I think it might have to do with https://github.com/documentationjs/documentation/issues/1617 but it also doesn't get fixed when commenting out the docs in the constructor. I'm going to play around a bit more before seeing if this is something more easily fixed by changing our inline docs conventions, or if it'd be easier to fix the in documentation.js directly.

[limzykenneth]

@davepagurek Was just going to ask, would you like me to put together a simple test document so that you can compare against a smaller generated file instead of the full one?

[davepagurek]

Are you thinking something like just a subset of p5?

[limzykenneth]

Yeah something like that, I'm extracting the p5.Framebuffer class at the moment to test things out. You can try it if you like: https://gist.github.com/limzykenneth/37b52c2bed719a211d7d9f24eb6d47cd

[limzykenneth]

@limzykenneth I hit a bit of a snag with detecting methods in ES6 classes correctly. It seems like in some cases like in p5.Framebuffer, documentation.js isn't correctly labeling its methods as being a memberof p5.Framebuffer. I think it might have to do with https://github.com/documentationjs/documentation/issues/1617 but it also doesn't get fixed when commenting out the docs in the constructor. I'm going to play around a bit more before seeing if this is something more easily fixed by changing our inline docs conventions, or if it'd be easier to fix the in documentation.js directly.

I did some digging and found that the problem is likely we are labeling the class as p5.Framebuffer but in the code the class name is just Framebuffer, as such the nested properties and methods (when their tags are corrected) belongs to Framebuffer and not p5.Framebuffer. I checked this by removing the name p5.Framebuffer from the @class tag and it correctly recognizes the pixels member after I made some correction to it.

I tried the @alias tag but it just reverts to previous behaviour. I'm not sure how to solve this just yet though.

[limzykenneth]

@davepagurek In the gist I have updated the test file to something that works 90% of the way. The main thing that doesn't work at the moment is modules. I'm not sure with documentation.js it is entirely possible without slightly annoying configs though, we may need to consider the structure we use to get around this.

[davepagurek]

Thanks for your pointers! Looks like I can get it to work when:

• take out the @constructor tag
• move the constructor above the class, as recommended in https://github.com/documentationjs/documentation/blob/master/docs/RECIPES.md#classes
• move properties out of the class itself, using @property and @for
• declare the class like p5.Framebuffer = class Framebuffer { ... } instead of assigning it afterwards
• remove @method methodName from ES6 class methods (it auto-detects those)

/**
 * An object that one can draw to and then read as a texture. While similar
 * to a p5.Graphics, using a p5.Framebuffer as a texture will generally run
 * much faster, as it lives within the same WebGL context as the canvas it
 * is created on. It only works in WebGL mode.
 *
 * @param {p5.Graphics|p5} target A p5 global instance or p5.Graphics
 * @param {Object} [settings] A settings object
 */
p5.Framebuffer = class Framebuffer {
  /**
   * Description here
   */
  begin() {}
}

The @class tag is also seems to be optional when assigned in this way.

So far I've just made the changes to p5.Framebuffer and the other things in that file, I'll go through some more later.

[davepagurek]

I think the last things to do are to implement conversion of constants, and to update other classes in the p5 source code to the constructor syntax that works (and to document that somewhere.) Out of the few top-level properties in the old json format, there are two that I think are safe to omit:

• I notice the warnings top-level array in the old JSON seems to just be warnings from yuidoc about unknown tags and such. Those are also in the .min.json version of the old format, were those getting included in the p5 bundle? If so, this could be a nice space-saving win to omit it.
• I haven't yet implemented the project and files top-level arrays from the old format, but I think we might not use these anywhere? Is that assumption correct?

[limzykenneth]

I notice the warnings top-level array in the old JSON seems to just be warnings from yuidoc about unknown tags and such. Those are also in the .min.json version of the old format, were those getting included in the p5 bundle? If so, this could be a nice space-saving win to omit it.

These can possibly be omitted as I don't think they are being used. The bundled stuff I think is just the parameter info in the parameterDoc.json file, built from data.json. It is used by FES. For min.json that will not be needed with the new website but it is just another step in Grunt handling it at the moment so nothing we need to worry about.

I haven't yet implemented the project and files top-level arrays from the old format, but I think we might not use these anywhere? Is that assumption correct?

I believe so, in any case we don't need to consider space saving for this file since it won't be served in the same way as the data.json file we have currently on the website. Feel free to include or omit them as you like.

[davepagurek]

I was looking into consts some more. I think yuidoc logged usages by seeing where they get used inside functions, since we don't specify within param types (currently anyway) specific constants, just the general Constant type.

It looks like Documentation.js doesn't give us the same sort of info on internal usage. I think that maybe the thing to do instead is replace uses of Constant with the specific constants? I tried updating the source code for endShape to do this for its CLOSE param, and beginShape() for all its shape modes. How does something like that sound to you?

[davepagurek]

Ok, new update: I think the initial version of the script handles all the things I want it to handle! (I still haven't converted all the uses of Constants, I'll do that if we agree on it, otherwise I can look into other ways of doing that.)

Next phase I think will be testing to double-check I haven't missed something. In current p5, I think that would include:

1. Making sure the reference still works
2. Making sure parameter validation still works

@limzykenneth is there an equivalent for (1) in the current build system without yuidoc, or do we mostly have to wait for the new site repo to exist to have a nice UI to browse and spot-check everything?

For (2), I think I'll start with some unit tests, assuming that some format changes (e.g. how constants, arrays, unions) might have broken it. But to make sure there are no other cases we missed, do you think the best way forward is to just make a build of p5, look for uses of _validateParameters in the source, and try out methods that use it?

[limzykenneth]

For constants I'm not exactly sure how documentation.js expects it to be handled (will need to look into it), I'm thinking if we can use type alias to give something readable for the constants so we basically still retain something similar to what we have?

Making sure the reference still works

If the new generated file have the same structure as the YUIDoc generated data.json file, we can make it a drop in replacement as long as the build script run this new script and replace the data.json file in the same location.

Making sure parameter validation still works

The proposed method will likely be the quickest to test although not necessarily comprehensive. There are some validate parameter unit tests peppered in the test files but for the dev-2.0 branch I set them all to be skipped as they don't quite work yet, and I don't like them being peppered everywhere, much rather there is one place where all the parameter validation get tested.

[davepagurek]

For constants I'm not exactly sure how documentation.js expects it to be handled (will need to look into it), I'm thinking if we can use type alias to give something readable for the constants so we basically still retain something similar to what we have?

I think if we use one big Constant type, the feature we would still be losing is per-constant usage. In current p5, the consts object has entries like this:

"NORMAL": [
  "p5.Image.blend",
  "p5.blend",
  "p5.textStyle",
  "p5.textureMode"
]

This looks like it's done by yuidoc by looking for references to those constants within the bodies of functions. We currently render these on pages for constants so I was trying to think about how to get something like this back again. From what I've seen, I don't think this is a feature Documentation.js tries to have. If we specify in the doc comments specifically which constants each method takes (or define a type for each one, like a BlendMode = BLEND | SUBTRACT | ADD | ..., that also works) then we can manually recreate this usage array by keeping track of what methods we see each constant in. I was thinking it might be beneficial for clarity in the docs if we're more specific in the type parameters?

In terms of just getting things working, we can continue to use Constant everywhere like we currently do and Documentation.js doesn't complain, but it just means we'll have empty usage arrays for all the specific constants. That could be the easiest way forward if we don't care too much about the "Used in..." section on reference pages for constants (and we always have the option of being more specific later.)

[limzykenneth]

I'm thinking more in terms of

type CLOSE = "close";

if it were typescript, so that when it comes to documenting it, it would be something like

@name endShape
@param [close] {CLOSE}

We can just go for anything that works for now. The main thing is to make sure that the documentation.js generated file have all the information we need one way or another so we know what the actual structure the inline documentation should be in. This can be more of a proof of concept at this stage anyway.

[davepagurek]

Ah ok, that is for explaining! I'm thinking of also having the @param tag look like that too, so it sounds like doing whatever we need to get closer to that would be good.

Since the existing Constant type doesn't break anything, for now I'll just move forward with testing the downstream uses of these types and then circle back to constants, possibly with some community help for converting the doc comments for them, since this seems related to the default values changes already being discussed for 2.0.

[davepagurek]

I've been testing this on the p5.js-website repo by running cp ../p5.js/docs/converted.json src/templates/pages/reference/data.min.json && npx grunt generate_enJSON. I noticed a bunch of little things that are now fixed!

One thing that remains, though, is that the ordering on p5js.org is different. @limzykenneth do you know how the ordering works currently? Is it just the order categories are added to the object here? https://github.com/processing/p5.js-website/blob/d311934b81eaed6b48161384493b14e3522f65cd/src/assets/js/reference.js#L4450 Do you think this is something we should try to match, or wait for the new site to use a different ordering method?

[limzykenneth]

@davepagurek I would need to look into the code in more detail but I think it may be related to the order things already are in in data.json that YUIDoc generates to some degree, that order itself likely has to do with some deterministic ordering of files and file traversal that is used.

That being said, we don't necessarily need to match if we are not deploying this to the current website. We can decide later if we need to sort at this step or the website can sort instead.

[limzykenneth]

Comparing the current website's reference section order with the src/* folder order, I would say the order here is basically alphabetical in terms of files and folders. If anything else attaches to existing sections, it will just be appended into that section without changing order.

[davepagurek]

I've made some changes so that I could run the FES checks successfully. This includes:

• Making npm run docs output parameterData.json
• Changing some of the test imports to load helpers and p5 correctly in vitest
• Fixing some bits in p5 causing tests to fail

With that, running npm run test test/unit/core/error_helpers.js works!

Once the existing tests ran, I also add tests to make sure that in addition to the existing {Constant} format of defining a constant type (which still works, but is less specific and no longer gives us a listing of which functions use which constants) we can also use FES when we specify constants directly in types (currently only used in endShape, where {CLOSE} is used directly.) That passes now too, so we should be safe to replace more constant param types with their specific constant options.

I also have a playbook for being able to update other test files, which mostly includes:

• Making sure p5 is imported
• Making sure any helpers (e.g. the chai helpers) are imported
• Making sure the math addons are set up
• Replacing setup, teardown, beforeAll, and afterAll with beforeEach/afterEach

[limzykenneth]

@davepagurek I just had a check on the converted JSON working with the current website and noticed that there are still some issues between them. The Constants section currently seems to be blank. The other probably more significant thing is that some functions related to event handling are missing (eg. mouseReleased()), these are probably due to methods that have the same name under another class, ie. p5.Element.prototype.mouseReleased() clash with the global mouseReleased().

[lindapaiste]

@param {Constant} mode is not valid JSDoc because Constant is not a type. When I run the p5 source code I get warnings about this.

@param {DEGREES|RADIANS} mode is not valid JSDoc because DEGREES etc is a variable and not a type.

@param {'degrees'|'radians'} mode is valid JSDoc but loses the association between the argument and the p5 constant.

If you wanted to have valid JSDoc, you could could add some @typedefs to the constants.js file

/**
 * @typedef {string} DEGREES
 * @typedef {string} RADIANS
 */

or more strictly

/**
 * @typedef {'degrees'} DEGREES
 * @typedef {'radians'} RADIANS
 */

and then it's ok to use @param {DEGREES|RADIANS} mode. In the first case mode has type string and in the second case it has type "degrees" | "radians" (one of these two exact strings only).

No idea...

• if that works with Documentation.js.
• if adding the @typedef to make it valid JSDoc is at all important to you
• what impact that has on generated TypeScript types

[davepagurek]

Update on the constants section missing: the current reference page only shows constants with examples, and I had forgotten to add the examples and alt properties to constants. That bit should be good now.

Going to look into the event handling ones soon!

@lindapaiste I think our migration path will be something like:

1. First just make sure we can output the same data format as before so our reference page can still work after switching to Documentation.js
2. Convert all arguments using @param {Constant} to the constant name (e.g. @param {DEGREES|RADIANS}), which will also be supported by the reference page
3. Add @typedefs to get closer to Typescript support (https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#typedef-callback-and-param)

[davepagurek]

Ok I found out what was going on with the event handling methods! Turns out it wasn't anything fundamentally wrong with the duplicate names, I just forgot to apply the rules from https://github.com/processing/p5.js/pull/6777#issuecomment-1928263993 to that class too. Now its properties and methods show up correctly.

[davepagurek]

Another update: I've added typedefs for all constants and replaced all usage of a generic Constant type with a union of specific constants! This also lets us get more specific tracking of where each constant is used:

For constants that @limzykenneth converted to symbols, right now I'm using {unique symbol} as the type since that seems to be the way Typescript-in-JSDoc has you do it. (I still have not actually run this through tsc though.)

[davepagurek]

@limzykenneth just resolved all the merge conflicts so that the docs stay in the expected updated format, lmk if this is good to merge!

[limzykenneth]

I'll merge for now and worry about it later if something breaks