Manage locals with same key

[ThibaultAndreis]

Describe the problem

Currently, when declaring locals, we can set several of them with the same key:

const config = ({
	loaders: [
		{
			locale: 'en',
			key: 'a',
			routes: ['/'],
			loader: async () => (await import('./en/home.json')).default,
		},
		{
			locale: 'en',
			key: 'a',
			loader: async () => (await import('./en/layout.json')).default,
		}
	]
})

However, only the last loaded translation is kept, the others are overridden, leading to unexpected results.

For exemple, with the files

// ./en/home.json
{
  "my text for home": "text for home"
}

// ./en/layout.json
{
  "my text for layout": "text for layout"
}

I can have two possiblites of final file:

//if layout was the slowest
local = {
	a: {
		  "my text for layout": "text for layout"
	}
}

//if home was the slowest
local = {
	a: {
		  "my text for home": "text for home"
	}
}

Without having a consistent result of which value is set and which is undefined.

The probleme comes from the function serialize
having this bit of code

const output = {
	...(acc[validLocale] || {}),
	[key]: data
};

Proposed solution

Allow merging translations with the same key.

It should be optin to avoid breaking current implementations

const config = ({
	identicalKeys: 'merge',
	loaders: []
})

with the default value being:

identicalKeys: 'override'

And modify the current serialize function to check of the config set

let output;
// If a value is set at the current key and identicalKeys is set to merge
if (acc[validLocale]?.[key] && identicalKeys === 'merge') {
	// Merge the existing identicalKey with the new data
	output = {
		...acc[validLocale],
		[key]: {
			...acc[validLocale][key],
			...data
		}
	}
} else {
	// Use the new data exclusively for the key
	output = {
		...(acc[validLocale] || {}),
		[key]: data
	}
}

We could use a function to make this cleaner and make output a const again.

Adding this would allow us to have more consistent results.

This would solve my use case which is having no key set

const config = ({
	loaders: [
		{
			locale: 'en',
			routes: ['/'],
			loader: async () => (await import('./en/home.json')).default,
		},
		{
			locale: 'en',
			loader: async () => (await import('./en/layout.json')).default,
		}
	]
})

// ./en/home.json
{
  "my text for home": "text for home"
}

// ./en/layout.json
{
  "my text for layout": "text for layout"
}

Which would give me the object

const local = {
	"my text for layout": "text for layout",
	"my text for home":  "text for home"
}

It is useful for us to not set a key, as it allows us to use the requested value as a clean fallback.
Giving us for example 'my requested text' and not 'home.text' when no translation is set, which is better for our
users.

This solution wouldn't prevent all problems itself, mainly because we can still have the following case:

const config = ({
	loaders: [
		{
			locale: 'en',
			key: 'home',
			routes: ['/'],
			loader: async () => (await import('./en/home.json')).default,
		},
		{
			locale: 'en',
			loader: async () => (await import('./en/layout.json')).default,
		}
	]
})

// ./en/home.json
{
  "homeText": "text for home"
}

// ./en/layout.json
{
  "home": "Home" // <- used for instance in a menu
}

The serialize function would have two objects that conflict, with different types:

// local from home.json
{
  "home": {
    "homeText": "text for home"
  }
}

// local from layout.json
{
  "home": "Home"
}

How should we manage this case?
Should we be more opinonated and decide which value to keep, or simply alert the user with a console.log that this is not managed ?

It also doesn't solve the case of two string having the same key; The returned value would still be inconsistent.

Alternatives considered

Disallow setting locales with the same key.

Probably the safest, as it would avoid any inconsistent behaviour but also reduicing the capability of this lib, making it not usable for same usecases (including mine).

Simply logging when a locale is overridden.

It would help understand the problem for newcomers, but wouldn't prevent it.

[ThibaultAndreis]

After checking a bit, I think we should disallow not setting any key / setting an empty string.
The key is used to check if we should load the translation file. It can cause existing translations to be loaded on direct access and not during client-side navigation. (I can provide an example project if necessary)

As an alternative for my use case, I think we could provide a way to disable the automatic dot notation (I can create a separate issue to discuss that).

We should, however, still provide a way to manage the same key problem exposed here.

[jarda-svoboda]

Hi @ThibaultAndreis!
Thanks for your great point for a discussion.
I’m wondering if we could use config.preprocess allowing to use an object instead of string..
For example:

{
    preprocess: {
        strategy: 'full',
        identicalKeys: 'merge'
    }
}

[ThibaultAndreis]

This seems like a logical modification to me, but it may require implementing it as a major release to respect semantic versioning and avoid disrupting other people's workflows.

We could make it this way, or just add the new code for now, without changing preprocess, limiting potential breakage to only the new code.

We can still make changes in a future release, if necessary. However, it might require more users to modify their configurations in the end.

How do you think we should pass the value of 'identicalKeys' to the serialization function?

Edit : If we include the change preventing from using empty keys, it would be a breaking change, so we can modify the config anyway I think