Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: supports selecting the overwrite order of merge messages #1896

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

supercpq
Copy link

Hello, I added this code to provide users facing this situation with more options: whether to replace the existing message on the i18n instance with the passed message.
And this way of writing can keep the function calls in the vue-i18n npm package unaffected.
The background reason is that I have an npm package and a Vue3 code base. When app.use this npm package, I passed the i18n instance of the Vue3 code base to the install function of the npm package, and then called mergeLocaleMessage, which caused the message of the npm package to overwrite the value of the same key in the Vue3 code base, but I hope that the message in the Vue3 code base has a higher priority, so I thought of this way.
I hope it can be adopted, thank you.

@BobbieGoede
Copy link
Member

BobbieGoede commented Jul 17, 2024

I don't quite understand the use case and implementation, this would basically loop through the messages object without doing anything when isRetainExistMessage is true, right? 🤔

@supercpq
Copy link
Author

I don't quite understand the use case and implementation, this would basically loop through the messages object without doing anything when isRetainExistMessage is true, right? 🤔
Sorry, I should make the following judgment only when the key is the same, instead of not calling it in all cases. I will modify it.

@BobbieGoede
Copy link
Member

I think you would get the desired result by calling mergeLocaleMessage again with an object containing only the messages you would like to preserve.

@supercpq
Copy link
Author

That means I need to call mergeLocaleMessage twice, the first time passing in what I need to merge, and the second time passing in what I originally wanted, to achieve the overwriting situation. This is indeed a solution, but if it can be directly supported from mergeLocaleMessage, I believe it will be more readable and have higher performance.

@supercpq
Copy link
Author

I resubmitted the code. When the key does not exist on des, it will be added.
Please reconsider this code

@supercpq
Copy link
Author

Compared with the original version:
Original version: When the key value is the same, the incoming message will overwrite the original message on the i18n instance
New version: When the key value is the same, if isRetainExistMessage=true is passed in, the original message on the i18n instance will not be changed
In other cases, the two boards are the same 😃

@BobbieGoede
Copy link
Member

Ideally you should be able to have some control over the priority/order of loading and setting messages in your project. In Nuxt I18n we rely on this behavior heavily, adding additional comparisons in the deepCopy function could have performance implications when merging very large message objects, or merging message objects often (not uncommon in our module).

Can you provide a minimal reproduction of your use case? I feel like there should be a better approach rather than modifying the deepCopy and mergeLocaleMessage signatures/logic.

That means I need to call mergeLocaleMessage twice, the first time passing in what I need to merge, and the second time passing in what I originally wanted, to achieve the overwriting situation. This is indeed a solution, but if it can be directly supported from mergeLocaleMessage, I believe it will be more readable and have higher performance.

If performance is a concern and you're unable to change the merge priority/order within the project, wouldn't it be possible to use getLocaleMessage to get the original messages, set the new messages using setLocaleMessage and passing the original messages to mergeLocaleMessage? This may look odd but this way you're looping over and merging the messages only once.

@kazupon kazupon added the Status: Need More Info Lacks enough info to make progress label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Need More Info Lacks enough info to make progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants