Add support for inline localized values for Builder #1659
Conversation
🦋 Changeset detectedLatest commit: 862d587 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit 862d587.
☁️ Nx Cloud last updated this comment at |
Deploying mitosis with
|
| Latest commit: |
862d587
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://03d6e70f.mitosis-9uh.pages.dev |
| Branch Preview URL: | https://korey-support-localization.mitosis-9uh.pages.dev |
mrkoreye
changed the title
There was a problem hiding this comment.
you forgot to add a changesets entry: https://github.com/BuilderIO/mitosis/blob/main/CONTRIBUTING.md#preparing-your-pr
| expect(mitosisJsx).toMatchSnapshot(); | ||
|
|
||
| const backToBuilder = componentToBuilder()({ component }); | ||
| expect(backToBuilder).toMatchSnapshot(); |
There was a problem hiding this comment.
I visually compared originalBuilder and backToBuilder using https://www.diffchecker.com, and found some discrepancies in the JSON, specially the structure of the children blocks.
Just wanted to give you a heads-up so you can confirm that the output is indeed what you expect it to be and that it doesn't need to be exactly equal to the original Builder jSON.
There was a problem hiding this comment.
Thank you for looking closely! I saw that too and assumed that was just part of the mitosis to builder and back process because no other previously written tests were breaking, but maybe I should recheck the changes I made 🤔
There was a problem hiding this comment.
I looked more closely and it seems like my changes are not the cause of this, and that putting the text node in a child when converting back to builder is the current mitosis behavior.
| if (path.startsWith(propertiesPrefix)) { | ||
| const key = path.replace(propertiesPrefix, ''); | ||
| element.properties = element.properties || {}; | ||
| (element as any).properties[key] = value; | ||
| } else if (path === 'linkUrl') { | ||
| (element as any).linkUrl = value; | ||
| } else if (path.startsWith(componentOptionsPrefix)) { | ||
| const key = path.replace(componentOptionsPrefix, ''); | ||
| if (element.component) { | ||
| element.component.options[key] = value; | ||
| } | ||
| } |
There was a problem hiding this comment.
you can probably use a set helper to do this (I suggest copy/pasting https://github.com/BuilderIO/builder/blob/main/packages/sdks/src/functions/set.ts instead of using the lodash version to keep bundle size low)
| if (path.startsWith(propertiesPrefix)) { | |
| const key = path.replace(propertiesPrefix, ''); | |
| element.properties = element.properties || {}; | |
| (element as any).properties[key] = value; | |
| } else if (path === 'linkUrl') { | |
| (element as any).linkUrl = value; | |
| } else if (path.startsWith(componentOptionsPrefix)) { | |
| const key = path.replace(componentOptionsPrefix, ''); | |
| if (element.component) { | |
| element.component.options[key] = value; | |
| } | |
| } | |
| set(element, path, value) |
it would be more future proof: you won't need to add more cases here as time goes on
There was a problem hiding this comment.
oh nice, will look at changing that
|
@samijaber updated |
Description
This is a WIP PR to support inline localization for the builder parser. Next step is to add support to the generator. The idea is that when we recognize a property value as a localized value, we set the mitosis value to the
Defaultand store the original localized value as a string on a$prefixed property. Then, (not completed yet) we will take the$prefixed values to create a localized value when converting back to Builder.Make sure to follow the PR preparation steps in CONTRIBUTING.md before submitting your PR:
yarn fmt:prettier.yarn test:updateyarn g:changesetand follow the CLI instructions. Alternatively, use the Changeset Github Bot to create the file.