tryggvigy / pseudo-localization

Dynamic pseudo-localization in the browser and nodejs
https://tryggvigy.github.io/pseudo-localization/hamlet.html
MIT License
140 stars 15 forks source link

psuedoLocalizeString function does not check if string is a string or null #6

Closed kittycatbytes closed 6 years ago

kittycatbytes commented 6 years ago

I get the following error: JavaScript runtime error: Unable to get property 'map' of undefined or null reference - index.js (44,1)

However this is resolved by changing your pseudoLocalizeString function to:

const psuedoLocalizeString = (string, options = { strategy: "accented" }) => {
    if (string && typeof string === 'string') {
        let opts = strategies[options.strategy];

        let pseudoLocalizedText = "";
        for (let character of string) {
            if (opts.map[character]) {
                const cl = character.toLowerCase();
                // duplicate "a", "e", "o" and "u" to emulate ~30% longer text
                if (
                    opts.elongate &&
                    (cl === "a" || cl === "e" || cl === "o" || cl === "u")
                ) {
                    pseudoLocalizedText += opts.map[character] + opts.map[character];
                } else pseudoLocalizedText += opts.map[character];
            } else pseudoLocalizedText += character;
        }

        // If this string is from the DOM, it should already contain the pre- and postfix.
        if (
            pseudoLocalizedText.startsWith(opts.prefix) &&
            pseudoLocalizedText.endsWith(opts.postfix)
        ) {
            return pseudoLocalizedText;
        }
        return opts.prefix + pseudoLocalizedText + opts.postfix;
    } else {
        return '';
    }
};

Could you kindly update to catch for this error? Or if you have an alternate suggestion please let me know. Thank you!

tryggvigy commented 6 years ago

Hey @kittycatbytes ! I assume you worked around the issue?

If you are working in an environment where where the string passed could be null you could wrap psuedoLocalizeString before using it. Something like myCustomPesudoLocalizeString.js that could look like:

import { pseudoLocalizeString as libPseudoLocalizeString } from 'psuedo-localization';

export function pseudoLocalizeString(string) {
  return (string && typeof string === 'string') ? libPseudoLocalizeString(string) : '';
}
kittycatbytes commented 6 years ago

Yes, we were trying to avoid doing it at the string level as we have a stringTables object that is accessed based upon the selected user language, and accessed many places in the app. This would mean we would have to alter each place where we access our stringTable in our code, which is nearly everywhere we use a string in our app. However, we decided we didn't want to pass our entire dom in for manipulation, because the library does not distinguish between characters within style, svg, or defs tags, which we do not want (potentially others). So we will need to go another route. Thank you for your help though, and great repo!

tryggvigy commented 6 years ago

@kittycatbytes

alter each place where we access our stringTable in our code

Of course I lack a lot of context so this might be totally infeasible but I wonder if, in development, you could wrap the stringTable object with a proxy that does the pseudo localization. That way I think you wouldn't need to touch any of the places where stringTable is accessed, only where it's created/exposed, which might be a single place?

// stringsTable.js
// rawStringsTable is the actual object

let stringsTable;
if(process.env.NODE_ENV === 'development') {
  stringsTable = new Proxy(
    rawStringsTable, 
    {
      // Default to empty string or what ever is needed
      get: (obj, prop) =>  prop in obj ? pseudoLocalizeString(obj[prop]) : ''
    }
  );
} else {
  stringsTable = rawStringsTable;
}

export default stringsTable;

because the library does not distinguish between characters within style, svg, or defs tags, which we do not want (potentially others)

This is something I haven't realized until now! Thanks for mentioning that. I think it makes total sense to have some kind of blacklist functionality to be able to exclude tags like the ones you mentioned.