Tandem name conventions are applied haphazardly.

[pixelzoom]

As discovered in phetsims/axon#398 ...

In some cases, PhET wants to ensure that tandem names follows a specific naming convention. For example, all radio buttons should have a tandem name that ends with "RadioButton".

How those conventions are applied is implemented inconsistently, and in some cases not at all. Specifically:

• tandem.name suffix is hardcoded in multiple places, instead of factored out (e.g. 'Property' in ReadOnlyProperty)
• No convention is checked in places where there is a defacto standard: Checkbox, Panel, AccordionBox, Dialog, "groups", most buttons, ...
• tandem.supplied is not checked in some places, resulting in failure with Tandem.OPT_OUT

So...

Is it desirable to add runtime checking of more tandem-name conventions? I think so. The alternative is to visually catch them in Studio, which is like looking for a needle in a haystack.

Should the implementation of those checks be similar? I think so. This will make adding additional checks easier, and make maintenance easier.

[pixelzoom]

• tandem.name suffix is hardcoded in multiple places, instead of factored out (e.g. 'Property' in ReadOnlyProperty)

In the above commits, I factored out static TANDEM_NAME_SUFFIX where that suffix was being duplicated.

There are a few others that I identified (by searching for endsWith) but was unable to address:

• ReadOnlyProperty duplicates suffix 'Property'. But attempting to factor that out to static TANDEM_NAME_SUFFIX resulted runtime error: Property.ts:13 Uncaught ReferenceError: Cannot access 'ReadOnlyProperty' before initialization
• Screen duplicates suffix 'Screen'
• RootTandem duplicates suffix 'Screen'. Making the tandem repo depend on joist (Screen) seems problematic.

[arouinfar]

Is it desirable to add runtime checking of more tandem-name conventions? I think so. The alternative is to visually catch them in Studio, which is like looking for a needle in a haystack.

I also think so. The less I need to manually review in Studio, the better.

[samreid]

I addressed the requests in the commits. There are still a few imperfections due to other constraints:

• Tandem cannot import from Screen, so the constant is defined in Tandem
• PhetioIDUtils has a duplicated block of constants that overlap with other things.

Ready for review.

UPDATE: I recalled this request:

Checkbox, Panel, AccordionBox, Dialog, "groups", most buttons, ...

Should this issue be a chip away for other occurrences too? Should we do that eagerly, or just as we are working in those areas?

[pixelzoom]

• No convention is checked in places where there is a defacto standard: Checkbox, Panel, AccordionBox, Dialog, "groups", most buttons, ...

Yes, this should be discussed/addressed. Anything that has a design convention should have a corresponding check in the code to ensure that the convention is followed.

[samreid]

All remaining work moved to side issues. @zepumph do you want to review this issue?

[zepumph]

I think a quick co-review would be the fastest here.

[zepumph]

• tandemSuffix Text/RichText should be Text. add tandemNameSuffix: 'Text' to Text/RichText scenery#1449****

[zepumph]

This blocks PhET-iO because tandem names will change for Text suffixing.

[zepumph]

In the next PhET-iO design meeting, we did a PSA about the new feature of PhET-iO elements called tandemSuffix. So if anyone finds a type, like Button, and not all tandem names end in "Button", we can add an assertion for that.

[zepumph]

renamed to tandemNameSuffix above (see https://github.com/phetsims/phet-io/issues/1855).

• We should most likely only assert about tandemNameSuffix when phetio validation is enabled. This seems like it will really bite the next person to try to instrument a pre-existing sim.

[zepumph]

@samreid, anything else here?

[samreid]

Everything looks great, nice work and thanks. Closing.