Double fix for 1395 issue

[lgordey]

@mgroenhoff @jamuhl
Now i feel guilty >_<
Initiative is really punishable :)

I hope this solves the problem #1398

[jamuhl]

Don't worry...you added value to this lib...contributions are highly appreciated 👍

I will check this tomorrow...

[jamuhl]

@mgroenhoff could you please review if this solves your issue?

[mgroenhoff]

No sorry, it does not. I've set up some quick tests to illustrate the problem:

const assert = require("assert");

require("./index").init({
    // debug: true,
    lng: "en",
    keySeparator: "→",
    nsSeparator: "§",
    pluralSeparator: "‗",
    contextSeparator: "»",
    resources: {
        en: {
            translation: {
                "foo $t(bar)": "translated foo $t(bar, {'a': 'baz'})",
                "foo $t(bar, {'a': 'a'}) a": "translated foo $t(bar, {'a': {{a}}})",
                "foo $t(bar, {'a': {{a}}}) a": "translated foo $t(bar, {'a': {{a}}})",
                "foo {{bar}}": "translated foo {{bar}}",
                "bar": "translated bar {{a}}",
                "bar1, bar2": "translated bar1, translated bar2",

                "foo $t(bar1, bar2)": "translated foo $t(bar1, bar2)",
                "foo $t(bar1, bar2, {})": "translated foo $t(bar1, bar2, {})",
                "foo $t(bar1, bar2, {'a': 'a'})": "translated foo $t(bar1, bar2, {'a': 'a'})",
                "foo $t(bar1, bar2, {'a': {{a}}})": "translated foo $t(bar1, bar2, {'a': {{a}}})",
            }
        }
    }
}, (_err, t) => {
    const t1 = "translated foo translated bar baz";
    assert.strictEqual(t("foo $t(bar)"), t1);
    assert.strictEqual(t("foo $t(bar, {'a': 'a'}) a", { a: "baz" }), t1);
    assert.strictEqual(t("foo $t(bar, {'a': {{a}}}) a", { a: "baz" }), t1);
    assert.strictEqual(t("foo {{bar}}", { bar: "$t(bar, {'a': {{b}} })", a: "baz" }), t1);

    const t2 = "translated foo translated bar1, translated bar2";
    assert.strictEqual(t("foo $t(bar1, bar2)"), t2); // <-- assertion
    assert.strictEqual(t("foo $t(bar1, bar2, {})"), t2);
    assert.strictEqual(t("foo $t(bar1, bar2, {'a': 'a'})"), t2);
    assert.strictEqual(t("foo $t(bar1, bar2, {'a': {{a}}})"), t2);
});

Changing the return statement into the following would fix the issue. Please note that this is a quick fix because it tries to see if the part after the last comma kinda looks like a JSON object and if it doesn't it'll put it back.

if (!/^\s*\{.*\}\s*$/.test(optionsString)) {
	key += "," + optionsString;
}

Again like i said in the original issue, the real fix for this would be to make the separation character used to separate the key and the options string configurable. As you can see in the above example i like the keys to match the values as much as possible and i use rarely used Unicode characters for other separation characters. For example the default value for nsSeparator is : which would mess up the keys big time.

I guess you could also refactor the entire handleHasOptions function and prevent deconstructing and reconstructing the key all the time (using lastIndexOf + slice i.s.o. split + pop + join) but i'll leave that up to you.

[jamuhl]

published in i18next@19.3.0