diff --git a/js/common/ChipperStringUtils.js b/js/common/ChipperStringUtils.js index e4f0abe68..af606a8f9 100644 --- a/js/common/ChipperStringUtils.js +++ b/js/common/ChipperStringUtils.js @@ -118,31 +118,17 @@ /** * Recurse through a string file and format each string value appropriately - * @param {Object.} stringsMap - if "intermediary", then recurse to - * find more nested keys + * @param {StringMap} stringMap * @param {boolean} isRTL - is right to left language * @public */ - formatStringValues: function( stringObject, isRTL ) { - for ( const stringKey in stringObject ) { - if ( stringObject.hasOwnProperty( stringKey ) ) { - - // This will either have a "value" key, or be an object with keys that will eventually have 'value' in it - const element = stringObject[ stringKey ]; - if ( element.hasOwnProperty( 'value' ) ) { - - // remove leading/trailing whitespace, see chipper#619. Do this before addDirectionalFormatting - // TODO: some a11y strings have leading/trailing whitespace purposefully, perhaps we should formalize that somehow, https://github.com/phetsims/chipper/issues/779 - element.value = element.value.trim(); - element.value = ChipperStringUtils.addDirectionalFormatting( element.value, isRTL ); - } - else { - - // Recurse a level deeper - ChipperStringUtils.formatStringValues( element, isRTL ); - } - } - } + formatStringValues: function( stringMap, isRTL ) { + ChipperStringUtils.forEachString( stringMap, ( key, stringObject ) => { + + // remove leading/trailing whitespace, see chipper#619. Do this before addDirectionalFormatting + stringObject.value = stringObject.value.trim(); + stringObject.value = ChipperStringUtils.addDirectionalFormatting( stringObject.value, isRTL ); + } ); }, /** @@ -150,7 +136,7 @@ * This method is called during requirejs mode from the string plugin and during the build via CHIPPER/getStringMap. * This method supports recursing through keys that support string nesting. This method was created to support * nested string keys in https://github.com/phetsims/rosetta/issues/193 - * @param {Object.} map - where an "intermediate" Object should hold nested strings + * @param {StringMap} map - where an "intermediate" Object should hold nested strings * @param {string} key - like `FRICTION/friction.title` or using nesting like `a11y.nested.string.here` * @returns {string|null} - the string value of the key, or null if the key does not appear in the map * @throws {Error} - if the key doesn't hold a string value in the map @@ -192,14 +178,19 @@ /** * This implementation function helps keep a better api for `forEachString`. * @param {string} keySoFar - as we recurse down, build up a string of the key separated with dots. - * @param {Object} map - string key map - * @param {function(key:string, {value:string})} func + * @param {StringMapNode} map + * @param {function(key:string, StringObject)} func */ const forEachStringImplementation = ( keySoFar, map, func ) => { for ( const key in map ) { if ( map.hasOwnProperty( key ) ) { const nextKey = keySoFar ? `${keySoFar}.${key}` : key; // don't start with period, assumes '' is falsey const stringObject = map[ key ]; + + // no need to support arrays in the string map, for example stringObject.history in locale specific files. + if ( Array.isArray( stringObject ) ) { + return; + } if ( stringObject.value ) { func( nextKey, stringObject ); } @@ -210,6 +201,24 @@ } }; + /** + * @typedef {Object} StringMapNode + * @property {StringMapNode} * - A key that stores a StringMapNode inside this one. + */ + /** + * @typedef {Object} StringObject + * An object that has a "value" field that holds the string. It can still include more nested `StringObject`s. + * Each StringMapNode should have at least one StringObject nested inside it. + * @extends {StringMapNode} + * @property {string} value - the value key is used in + */ + /** + * @typedef {Object.>} StringMap + * @extends {StringMapNode} + * A string map can be either a flat map of StringObject (see the output of CHIPPER/getStringMap), or can be a nested + * Object with StringObjects throughout the object structure (as supported in English JSON string files). + */ + // browser require.js-compatible definition if ( typeof define !== 'undefined' ) { define( function() { diff --git a/js/requirejs-plugins/string.js b/js/requirejs-plugins/string.js index 699156fa5..e00e6ef41 100644 --- a/js/requirejs-plugins/string.js +++ b/js/requirejs-plugins/string.js @@ -28,10 +28,15 @@ define( require => { // constants // Cache the loaded strings so they only have to be read once through file.read (for performance) - // Object.>} + // Object.} - see ChipperStringUtils for typedef of StringMap // Where stringValueObject is the value of each key in string json files. const cache = {}; + // {Object.} - keep track of actions to trigger once the first load comes back for that + // url. This way there aren't many text.get calls kicked off before the first. + // can come back with text. + const urlsCurrentlyBeingLoaded = {}; + // {string|null} - See documentation of stringTest query parameter in initialize-globals.js const stringTest = ( typeof window !== 'undefined' && window.phet.chipper.queryParameters.stringTest ) ? window.phet.chipper.queryParameters.stringTest : @@ -50,16 +55,28 @@ define( require => { if ( cache[ url ] ) { callback( cache[ url ] ); } + else if ( urlsCurrentlyBeingLoaded[ url ] ) { + + // this url is currently being loaded, so don't kick off another `text.get()`. + urlsCurrentlyBeingLoaded[ url ].push( () => callback( cache[ url ] ) ); + } else { + urlsCurrentlyBeingLoaded[ url ] = []; + // Cache miss: load the file parse, enter into cache and return it text.get( url, loadedText => { + const parsed = JSON.parse( loadedText ); const isRTL = localeInfo[ phet.chipper.queryParameters.locale ].direction === 'rtl'; - ChipperStringUtils.formatStringValues( parsed, isRTL ); cache[ url ] = parsed; + + // clear the entries added during the async loading process + urlsCurrentlyBeingLoaded[ url ] && urlsCurrentlyBeingLoaded[ url ].forEach( action => action && action() ); + delete urlsCurrentlyBeingLoaded[ url ]; + callback( cache[ url ] ); }, errorBack, { accept: 'application/json' } ); }