TickLabelSet caching can result in incorrect labels.

[pixelzoom]

Discovered while working on https://github.com/phetsims/bamboo/issues/64 ...

TickLabelSet implements a form of caching for its tick labels. If a label had been previously created for a numeric value, that label is reused. Otherwise, the createLabel function is called to create a new label. Here's the relevant code:

       const label = this.labelMap.has( modelCoordinate ) ? this.labelMap.get( modelCoordinate )! :
                      this.createLabel ? this.createLabel( modelCoordinate ) :
                      null;

The problem here is that createLabel may involves other factors that TickLabelSet does not know about, and a new label may indeed need to be created for the numeric value, regardless of whether a label already exists.

For example, suppose we have ticks labeled '7 m' and '7 nm'. Both ticks have the value 7, but the labels need to be different, not reused.

Another (real) example occurs in FMW DiscreteHarmonicsChartNode:

      xTickLabelSetOptions: {
        createLabel: value =>
          TickLabelUtils.createTickLabelForDomain( value, X_TICK_LABEL_DECIMALS, harmonicsChart.xAxisTickLabelFormatProperty.value,
            harmonicsChart.domainProperty.value, harmonicsChart.fourierSeries.L, harmonicsChart.fourierSeries.T )
      },

The labels here are specific to the selected domain (space, time, space & time). If 2 domains happen to share the same numeric value for a tick mark, then a stale label will be reused by TickLabelSet.

[pixelzoom]

@samreid and I discussed this. Rather than removing the caching feature, it seems better/safer to add an option to turn it on/off. So the plan is:

• [x] Add option cachingEnabled: boolean with good documentation.
• [x] Default to cachingEnabled: true because that is the current behavior, and sims may be relying on it.
• [x] Use cachingEnabled: false in FMW DiscreteHarmonicsChartNode.ts

I'll do the work, then ask @samreid to review.

[pixelzoom]

@samreid please review.

See the checklist in https://github.com/phetsims/bamboo/issues/65#issuecomment-2518683922.

https://github.com/phetsims/fourier-making-waves/commit/f6715561aa3716a1d9cf9bc33d48d4b82c165d10 is the change to TickLabelSet.

https://github.com/phetsims/bamboo/commit/5251e9e13fa6831d32f05b8de70b96720c2fa4ef is the opt-out of caching in FMW.

In addition to the checklist, note this improvement for readability: https://github.com/phetsims/bamboo/commit/5251e9e13fa6831d32f05b8de70b96720c2fa4ef#diff-809d7cf30752c0e3a166602cba3422dd411ad22bc9c08e6f54f15679cb1c30f3L151

If all looks OK, feel free to close.

[pixelzoom]

By the way... Note that I took the simpliest, least invasive approach to adding the cachingEnabled options. When cachingEnabled: false, there are several things that don't need to be created: this.labelMap, const used, const removed,... But conditionally creating those things would have been an entirely new code path, which I chose not to do. So with cachingEnabled: false, this.labelMap exists and is fully emptied on each call to update.