Seems like we already started in this direction:

One question arises for conflicts like we have pc.ShaderPass and pc.SHADERPASS_? Are you going to squeeze them into the ShaderPass class or still open to pc.SHADERPASS naming as enum object e.g. or any other ideas?

Using constants vs strings directly, has minimal performance impact, and mainly should be used as an organizational tool, just for auto-complete really and docs.

So how docs look with Enums?

For the enum I have above, it generates:

On the left, you get a single entry of a group of constants.

Looks great!

Just the order probably needs adjustment, to ensure classes - are the first.

Will there be name collisions between enums and classes?

Yeah, I agree enums should come after classes. TypeDoc let's you configure the order - this is the default.

If there's a name collision, we can tweak names.

The main problem I'm seeing is that when you write JSDocs and have a function that takes a TextureProjection enum say, you can't do:

@param {TextureProjection}

Instead you have to define an additional type:

@typedef {TextureProjection.Cube|TextureProjection.Equirect|TextureProjection.None|TextureProjection.Octahedral} TextureProjectionType

Then you can do:

@param {TextureProjectionType}

It's quite ugly.

It's probably either that or:

@param {('rect'|'circle'|'ellipse')} - which is what you try to avoid with constants in the first place?

Exactly. 😬

I think this is a clear area where TypeScript really delivers on something JavaScript struggles with.

I think this is a clear area where TypeScript really delivers on something JavaScript struggles with.

It is more IDE issue rather than language issue.
TS - helps as much as good JSDoc does or any other ways of instructing IDE about meanings.

True, a lot of the pain is coming from varying levels of JSDoc support in IDEs.
But at the same time, it isn’t only an IDE issue. TypeScript’s language-level features and compiler checks add an extra layer of safety and consistency that JSDoc alone can’t match.

ValueOf is an established type function to turn type objects into unions:

TS PLAYGROUND

So we would simply have:

/**
 * Texture projection modes for storing texture data. These define how the texture's data is
 * interpreted or sampled, such as cubemap or equirectangular projections.
 *
 * @readonly
 * @enum {ValueOf<typeof TextureProjection>}
 * @category Graphics
 */
export const TextureProjection = Object.freeze({
    /** No specific projection format is used. */
    None: 'none',

    /** Cubemap projection format. */
    Cube: 'cube',

    /** Equirectangular projection format. */
    Equirect: 'equirect',

    /** Octahedral projection format. */
    Octahedral: 'octahedral'
});

(still the question around collisions, I found at least pc.Asset and pc.ShaderPass, potentially more, everything I can think of looks sort of ugly like pc.ASSET.ANIMATION OTOH capital letters mark defines/constants since 1970 in C programming)

But at the same time, it isn’t only an IDE issue. TypeScript’s language-level features and compiler checks add an extra layer of safety and consistency that JSDoc alone can’t match.

Yes, and a huge quantity of inconveniences and limitations in other places.
There is no silver bullet.

Also, why properties of enum are from Capital letter instead of camelCase?

@kungfooman Wow, cool! I think that could work. We can handle name collisions. For example, we could do SHADERPASS_ to ShaderPassId. And ASSET_ to AssetType.

@Maksims We can pick the convention for capitalization. What about:

TextureProjection.CUBE

I like this because:

• I think the enum name should use the same convention as a class
• The enum values obviously look like enums if all capitalized (and match the naming of static constants on classes such as Vec3.ZERO).

Hmm. @kungfooman, let's say I define MouseButton:

/**
 * Mouse buttons.
 *
 * @readonly
 * @enum {ValueOf<typeof MouseButton>}
 * @category Input
 */
export const MouseButton = Object.freeze({
    /** No buttons pressed. */
    NONE: -1,

    /** The left mouse button. */
    LEFT: 0,

    /** The middle mouse button. */
    MIDDLE: 1,

    /** The right mouse button. */
    RIGHT: 2
});

If I type a function param as MouseButton, I get:

Can't we get MouseButton to be shown as the type instead?

This works:

/**
 * Mouse buttons.
 *
 * @readonly
 * @enum {-1 | 0 | 1 | 2}
 * @category Input
 */
export const MouseButton = Object.freeze({
    /** No specific projection format is used. */
    NONE: -1,

    /** The left mouse button. */
    LEFT: 0,

    /** The middle mouse button. */
    MIDDLE: 1,

    /** The right mouse button. */
    RIGHT: 2
});

This generates the following types:

/**
 * *
 */
export type MouseButton = -1 | 0 | 1 | 2;
/**
 * Mouse buttons.
 *
 * @readonly
 * @enum {-1 | 0 | 1 | 2}
 * @category Input
 */
export const MouseButton: Readonly<{
    /** No specific projection format is used. */
    NONE: -1;
    /** The left mouse button. */
    LEFT: 0;
    /** The middle mouse button. */
    MIDDLE: 1;
    /** The right mouse button. */
    RIGHT: 2;
}>;

that looks great!
personally, I would not use all caps, like MouseButton.NONE but MouseButton.None.
I guess I used that in c++ / c# for too many years. All caps are used for preprocessor defines only.

I'm OK with going with Pascal case. @kungfooman, seems like you agree with @mvaligursky? What do you think @Maksims?

Yep, I'm also leaning more to the serene/non-screaming MouseButton.None!

I tried to replicate your TypeDoc issue, but strangely enough it works on my side. I just added this to src/index.js:

/**
 * Mouse buttons.
 *
 * @readonly
 * @enum {ValueOf<typeof MouseButton>}
 * @category Input
 */
export const MouseButton = Object.freeze({
    /** No buttons pressed. */
    NONE: -1,

    /** The left mouse button. */
    LEFT: 0,

    /** The middle mouse button. */
    MIDDLE: 1,

    /** The right mouse button. */
    RIGHT: 2
});


/**
 * @param {MouseButton} mb - The mouse button.
 */
export function test(mb) {
    console.log(mb);
}
test(MouseButton.MIDDLE);

/**
 * @param {MouseButton} mb - The mouse button.
 */
export class TestClass {
    /**
     * @param {MouseButton} mb - The mouse button.
     */
    test(mb) {
        console.log(mb);
    }
}
export const testClass = new TestClass();
test(MouseButton.MIDDLE);

Typedoc output:

So I'm a bit puzzled about that. In any case, if I can reproduce, it seems just like small fix (probably as TypeDoc plugin or a little regex rewrite). Do you mind making a draft PR with what you have already, so I may get a 100% reproduction?

@kungfooman Check this out: #7630

I believe enums should differentiate from Classes (PascalCase), as we have exclusively been using PascalCase for classes only.
This helps readability and to know what is what.

For object properties, camelCase is used.
While for constants UPPER_CASE was used.

1. MOUSE_BUTTON.MIDDLE - is hard to read, and dot gets lost.
2. MouseButton.Middle - not sure what is what, is MouseButton a class and Middle a static property pointing to another class? Can I instantiate MouseButton.Middle?
3. MouseButton.middle - reads like a property. This also can be interpreted as a static properties of a class - which is very similar to enums in their meaning and functionality.
4. MouseButton.MIDDLE - MIDDLE - is clearly a constant, MouseButton - can be mixed up as a class. If same naming convention will be used for Enums as for Classes, then this is clearly allows to understand what is Enum and that we are accessing its constant.

I personally believe that 1st option is weird and not common in JS world, while 2nd option is confusing as it contradicts with property naming convention (camelCase).
If we introduce Enums, as PascalCase, then its property should be either following property convention (camelCase, option 3) or constant convention (UPPER_CASE, option 4). Also, when using autocomplete, it will clearly be understandable that this is a constant if UPPER_CASE is used.

Will not mention other popular engine, but their naming convention for constants is mixture of different conventions and looks messy.
And the other most popular engine uses Enum.CONSTANT convention.

Just checked Babylon. They do 2. One of the key reasons for this is because it's a TypeScript coding convention and Babylon is, of course, written in TS. Just look at the official TS docs that use this style.

As you have probably noticed, the PlayCanvas codebase has been gradually migrating to a very TS-like structure over the past few years. First we did classes, then modules, then class fields. And we've been incrementally tightening up the types (using JSDoc) over an extended period. I think making our enums appear familiar to TS-devs is a positive step. And should it ever be the case the the engine does switch to TS, well, this would be one other thing that trivially carries over. I totally understand you're not personally a fan of TS, but a sizeable proportion of PlayCanvas devs do use TS, so all of this should be taken into consideration.

I also just took a quick look at Three. It just does top-level constants like ours except using Pascal case. It's not a massive problem for them to have top-level constants because they have a lot fewer than we do, so we benefit a lot more from grouping them into enums objects, simplifying our top-level API surface area.

I think it's important to remember that Intellisense should be a big help here. Autocompletion (plus AI copilots) should make this something you don't have to think too hard about.

As you have probably noticed, the PlayCanvas codebase has been gradually migrating to a very TS-like structure over the past few years. First we did classes, then modules, then class fields. And we've been incrementally tightening up the types (using JSDoc) over an extended period. I think making our enums appear familiar to TS-devs is a positive step.

ES6 had a proposal freeze on classes when TypeScript wasn't even released (2011 vs TS release 2012). So what you look at as "TS-like structure" is plain old JavaScript?

I like the practical JS coding side: Every UI developer sooner or latest has to iterate enums. Currently there is rather ugly code like this:

https://github.com/playcanvas/model-viewer/blob/ef6fa9e196841ec6fa389c11f85a322dfe2e33af/src/viewer.ts#L1545-L1558

                    <Select
                        label='Tonemap'
                        type='string'
                        options={['None', 'Linear', 'Neutral', 'Filmic', 'Hejl', 'ACES', 'ACES2'].map(v => ({ v, t: v }))}
                        value={props.observerData.camera.tonemapping}
                        setProperty={(value: number) => props.setProperty('camera.tonemapping', value)} />

https://github.com/playcanvas/model-viewer/blob/ef6fa9e196841ec6fa389c11f85a322dfe2e33af/src/ui/popup-panel/panels.tsx#L44

    setTonemapping(tonemapping: string) {
        const mapping: Record<string, number> = {
            None: TONEMAP_NONE,
            Linear: TONEMAP_LINEAR,
            Neutral: TONEMAP_NEUTRAL,
            Filmic: TONEMAP_FILMIC,
            Hejl: TONEMAP_HEJL,
            ACES: TONEMAP_ACES,
            ACES2: TONEMAP_ACES2
        };

        this.camera.camera.toneMapping = mapping.hasOwnProperty(tonemapping) ? mapping[tonemapping] : TONEMAP_ACES;
        this.renderNextFrame();
    }

You can take those values straight via Object.keys(pc.WhateverEnum) after your PR is finished and people can refactor their code to look less like an exercise in bookkeeping 👍

I have created a spreadsheet of proposed renaming:

https://docs.google.com/spreadsheets/d/1EsIGuGBr4Btlkwqx-q3EdVThkdYTofl_IhRT0eI1BgM/edit?gid=0#gid=0

Anyone should have comment access so please log your suggestions.

So it looks like we be replacing 619 constants for 94 distinct new enum types. Quite a dramatic reduction in top-level API symbols.

As you have probably noticed, the PlayCanvas codebase has been gradually migrating to a very TS-like structure over the past few years. First we did classes, then modules, then class fields. And we've been incrementally tightening up the types (using JSDoc) over an extended period. I think making our enums appear familiar to TS-devs is a positive step.

ES6 had a proposal freeze on classes when TypeScript wasn't even released (2011 vs TS release 2012). So what you look at as "TS-like structure" is plain old JavaScript?

Well, obviously the engine is just "plain old JavaScript". But it is now written in a form of JavaScript that maps extremely closely to the equivalent TypeScript code. For the most part, the only difference now is that JSDoc types would drop down to inline types. This was absolutely not the case a few years ago.

Anyone should have comment access so please log your suggestions.

Mostly looks fantastic, but I added few suggestions.

Just to point it out: if we would follow standard JS style:

We could even further decrease the top-level API symbols by dropping the "AssetType" and the supposed "ShaderPassId" (that didn't make it in the spreadsheet yet or do you want to merge it into the class?). I like the clear/pure/distinct/practical enum object approach most still.

Well, obviously the engine is just plain old JavaScript. But it is now written in a form of JavaScript that maps extremely closely to the equivalent TypeScript code.

Yes, JSDoc was definitely an unruly mess with many different ways and limitations, so tsc streamlined it. 🚢 We took the best out of it and still retain the freedom of pure JS/ESM. 😅

We could even further decrease the top-level API symbols by dropping the "AssetType" and the supposed "ShaderPassId" (that didn't make it in the spreadsheet yet or do you want to merge it into the class?). I like the clear/pure/distinct/practical enum object approach most still.

True, we could move constants into relevant classes. We did that with events already, but I think it makes more sense for events since it documents the events fired by a specific class. For the other constants, I agree with you that a distinct enum would be better.

Yes, JSDoc was definitely an unruly mess with many different ways and limitations, so tsc streamlined it. 🚢 We took the best out of it and still retain the freedom of pure JS/ESM. 😅

Agreed!

True, we could move constants into relevant classes.

I would not, that would make a circular references much larger problem.

Just checked Babylon. They do 2. One of the key reasons for this is because it's a TypeScript coding convention and Babylon is, of course, written in TS. Just look at the official TS docs that use this style.

Please check out their docs well, as cherry picking is not reasonable here.
They use mixture of various conventions:

1. top level constants mostly UPPER_CASE, with mixture of UPPER_Pascal, and some PascalCase.
2. Enums are using mixed convention also: PascalCase.UPPER_CASE and PascalCase.PascalCase, and it seems like 50/50 mixture.

I also just took a quick look at Three. It just does top-level constants like ours except using Pascal case. It's not a massive problem for them to have top-level constants because they have a lot fewer than we do, so we benefit a lot more from grouping them into enums objects, simplifying our top-level API surface area.

For Threejs it is a mixture also:

1. Top level constants use PascalCase
2. And they do have enums, and they use: PascalCase.UPPER_CASE for it.
3. Threejs does not use PascalCase.PascalCase for enums.

So lets please not cherry pick specific examples, and look at their source code as a whole.
As I've said before: these popular engines, don't have a strong rule on enums or top-level constants, and use mixture of different conventions. It is clear that they both use PascalCase.UPPER_CASE in various places. As well as few other conventions more unique to them individually.

I think it's important to remember that Intellisense should be a big help here. Autocompletion (plus AI copilots) should make this something you don't have to think too hard about.

I totally disagree here. As when you are reading someones code, you want to have good sense on what is what.
When writing code, people use different tools, and they have different behaviours. Some might use Notepad++ still. And code should be clean, consistent and easy to read, regardless of IDE assistance.

It is important to pick one convention and stick to it. And currently constants use UPPER_CASE. We can help existing users as well as new users by using that convention as it is unique and specific for constants. Not mixed with Classes or anything else.

Since we are already refactoring I would also like to throw in another idea:

Currently some enums follow this naming convention:

  "BODYTYPE_",
  "DEVICETYPE_",
  "ELEMENTTYPE_",
  "SKYTYPE_",
  "SPRITETYPE_",
  "SSAOTYPE_",
  "TEXTURETYPE_",
  "XRTYPE_"

You can find more via:

Object.keys(pc)
    .filter(_ => _.includes("TYPE"))
    .map(_ => _.split('_'))

Due to name collisions with existing classes, we need AssetType aswell (same for Curve, ShaderPass etc.)

B-b-b-bbut what if we would simply add Type prefix/suffix to everything? This would:

1. Prevent future naming collisions (maybe someone wants a class GamePad? Too bad, that's an enum already... API BREAK!)
2. May mitigate @Maksims concerns because at least you got the "Type" marker on everything now?

If we think this Type tagging further, it could also end up like:

// Could also be pc.Enums e.g.
pc.enums.Body.Static;
// Compared to old:
pc.BODYTYPE_STATIC;

For top-level API surface area minimisation, this would be the absolute best so far. But of course the question remains: what are we even maximizing for.

we would simply add Type prefix/suffix to everything

We could. But then we could add Class suffix to every class as well. I think the collisions are rare, we don't need to particularly consider naming to avoid them. In some cases, we'll use Type, but it would read strange if we had it for all enums, for example BlendFactorType, BlendEquationType, CubemapFaceType.

To me it seems pretty clear that Something.Name is enum. If the first character after the '.' is in caps, it's an enum, otherwise it's a member. Syntax highlighting also gives you a different color. I'm not sure anything else is needed.

To me it seems pretty clear that Something.Name is enum. If the first character after the '.' is in caps, it's an enum, otherwise it's a member. Syntax highlighting also gives you a different color. I'm not sure anything else is needed.

Is Something a class? Can it be instantiated? Will users expect to look for constants in classes instead of enums in top-level?

I would avoid adding "Type" to things, as it makes it too long.
It's more the second (after the dot) value access, that can confuse. But if it is simply: CubemapFace.FRONT - that is very obvious that it is accessing a constant.

Uppercase Something.VALUE indicates a static class constant for me, so using this feels consistent, though I've no strong opinion either way.

@Maksims I agree that Something is vague; can we rely on IDE Intellisense here (enum icon is visually distinct)? Otherwise SomethingType is an option.