test(message-loader): cover message loader in integration tests

[dogayuksel]

Summary

Modifies build instructions on the pipeline to cover important paths related to recently added message loader functionality. (#3496, #3511)

Description

Plugin for vite

We enable message loader on playground build. Playground application already has translations, so a successful build means that 'message loader plugin' has been used.

Loader for webpack

We enable the message loader on custom application template builds. Here, we don't have translations, so we add a simple script to simulate a translation workflow just before the build. 'Mock translate i18n messages for the build', copies core.json to en.json, which in return gets picked up by the message loader to be complied and bundled up.

So far message compilation has been done on the simple KEYVALUE format. ~In order to check for STRUCTURED_JSON format compilation, we change the message structure on starter-typescript template. Similar to normal starter template, translation workflow is simulated just before the build, in order to cover 'message-loader' compiling and bundling messages in STRUCTURED_JSON format.~

~This approach should have the added benefit of demonstrating app-kit's capabilities to work with different message formats, however if you believe this could confuse the users, we can adjust the mock translation script to do the conversion just before the build before the mock translation workflow.~

In order to check for STRUCTURED_JSON format compilation, we format and override the core.json file for the 'starter-typescript' ~application~ custom-view template in the mock translation script that runs just before the build. Afterwards, core.json is copied to en.json, which later on gets picked up by the message loader during the build to get compiled.

Next Steps

With sufficient automated coverage through changes above, we hope to proceed to convert 'opt-in' behaviour to 'opt-out', renaming 'ENABLE_I18N_MESSAGE_COMPILATION' to 'DISABLE_I18N_MESSAGE_COMPILATION' and make necessary adjustments.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
merchant-center-application-kit-components-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 28, 2024 9:59am

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 32f3c041837be6bd45f09de14eea1727481b5d48

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dogayuksel]

What we can maybe do is to reformat an starter messages on the go as part of the test template CI action. I can imagine a parameter we can use at that action level which would trigger the refactor of the message file only in the context of the test.

@CarlosCortizasCT Then I will add another branch to scripts/mock-translate-i18n-messages.mjs to override the core.json as STRUCTURED_JSON for when template is 'typescript-starter' before copying it to en.json. This way we build with STRUCTURED_JSON without modifying the template.

Something I'm also wondering is how we are covering the new functionality in the integration tests. Is the assumption that if the starter applications render the same text, the loader is working fine?

Yes, given that en.json exists during build, it will be bundled up and will be used during e2e tests. Here is the sanity check on this commit with the failing e2e tests, indeed 'translated' keys are used during e2e. https://github.com/commercetools/merchant-center-application-kit/pull/3528/commits/7b7f9c745e59365e28b451a9e243e5018dbf6170

[dogayuksel]

@CarlosCortizasCT Do we think we need a changset here too?

[CarlosCortizasCT]

@CarlosCortizasCT Do we think we need a changset here too?

I don't think we need it for this PR.