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

Support for published package language vendor files #177

Merged
merged 6 commits into from
May 27, 2024

Conversation

Haruki1707
Copy link
Contributor

This update addresses an issue where a 'php_vendor.json' file is generated but not utilized, as it is not imported by the plugin resolver.

Changes

  • Removed the generation of the 'php_vendor.json' file from the published package language files.
  • Merged the content of 'php_vendor.json' directly into the actual language files used by the resolver.

@xiCO2k
Copy link
Owner

xiCO2k commented May 22, 2024

Hey @Haruki1707 thanks for the PR, can you add a test for this one?

Thanks.

@Haruki1707
Copy link
Contributor Author

Sure, I’ve added a test for it. Feel free to review and make any modifications as needed, since I’m not very experienced in testing.

@Haruki1707
Copy link
Contributor Author

I found some improvements, such as moving the merge logic from prepareExtendedParsedLanguage to parseAll, where I have direct access to the folder name (or lang) used to determine the vendor translation keys that need to be merged into a specific lang.

Additionally, moving part of the complex merging logic to a separate function makes the parseAll code easier to understand.

@Haruki1707
Copy link
Contributor Author

Hi @xiCO2k,

I have tested these changes in a real Laravel project, and everything seems to be working as expected without any breaking changes to the latest version. I hope this can lead to a new version release soon, as it is very helpful when working with package languages.

Thanks.

@xiCO2k
Copy link
Owner

xiCO2k commented May 24, 2024

Thanks for that will review it during the weekend

@Haruki1707
Copy link
Contributor Author

Haruki1707 commented May 26, 2024

Sorry, this commit was meant to be part of another PR, but I misclicked submit on main. It addresses a problem with the tests that can occur randomly. Tests within a "test suite" are run synchronously, but different test suites are run asynchronously. This can sometimes cause translate.test and loader.test to interfere with each other, as happened in the latest test workflow.

This commit isolates the folder that loader.test uses to prevent it from interfering with translate.test. It achieves this by generating a copy of the fixtures folder and using it exclusively for all the tests inside loader.test.

@xiCO2k
Copy link
Owner

xiCO2k commented May 26, 2024

Thanks for that. One question @Haruki1707 if we have the same thing just in php the key will be the same?

Thanks.

@Haruki1707
Copy link
Contributor Author

@xiCO2k yes, it will be the same key as in PHP. Here is an example from a package I'm using and how the key is structured:

if (Auth::user()->confirmTwoFactorAuth($request->code)) {
    return back()->with('success', trans('two-factor::messages.enabled'));
}

As you can see, the structure of the key for packages is package-name::file.translation_string.

Currently, without this PR, what gets generated in php_vendor.json is package_name.lang.file.translation_string. If you check line 83 in the commits of loader.ts, you'll see that the .folder. part (which represents the language) is replaced by ::. This makes it match the PHP key format package-name::file.translation_string and then injects it into the corresponding php_lang.json.

[key.replace(`.${folder}.`, '::')]: value

@xiCO2k xiCO2k merged commit 565faad into xiCO2k:main May 27, 2024
1 check passed
@xiCO2k
Copy link
Owner

xiCO2k commented May 27, 2024

Fixed on v2.7.7.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants