MESDK-103 - v2 createMessageBundle() and example mocks #2795
Conversation
|
|
general notes:
Why not pass messages to
|
|
Why are we suddenly using a serialized pattern in
|
|
@jldec Could you address the above question in a LOOM (or written maybe better)? I agree with Loris&Samuel on the observations. |
|
@samuelstroschein @LorisSigrist This is not intended to provide an alternative string format for messages. My intent was to help us work on v2 persistence with existing project data (like the monorepo website), and to start experimenting with utility functions to manipulate v2 MessageBundles without having to operate directly on the AST. |
🙏 i like that better. I have modified the signatures to use an args object and pass an array of
I was thinking we might use that for migrating messages from existing i18next projects, but we can revisit that later. For now only {var} is supported.
👍 changed |
I'll do a loom as soon I can actually demo something useful The goal is supprt v2 MessageBundles with persistence in a way which is similar to what I demo'ed in https://www.loom.com/share/9f6e47997a784728a492c17e00ee030b (NOTE that video still shows persitence with the legacy internal Message ast) |
I was wondering about this, too, and thought you and @jldec dismissed my arguments against a serialized format. I am asking again if a serialized format is needed now/if the utilities are even needed. Isn't it OK if you manually type some messages for your tests for now @LorisSigrist until we get clarity on what "good" utilities will look like and if we even need them? |
I'm on your side here, I was also surprised to see a serialized format.
Yes, writing the AST fine for the moment. In the long term it will become a maintenance burden IMO. It's a bit like writing half your tests in byte code. It's the wrong level of abstraction for a lot of the tests. Many of paraglide's tests are "take this message, run it through the complier & see if it renders correctly". Introducing an abstraction here will eventually be necessary, but can wait until we have more experience. For now writing the AST is fine. |
|
@samuelstroschein I'd prefer to use createMessageBundle() and createMessage() in tests (like load-test) because it will simplify the changes required as we transition to v2 persistence. It's always better for tests like that to use more realistic data (which is possible but harder to do with hand-rolled AST). I agree that introducing the full MF2 string format dependency now, would mean commiting to that while our requirements are still in flux, so not doing that now. I am also nervous about the AST api changing again, and would like to wrap that in an api layer for apps instead of just opening it up directly - but we will have other opportunities to do that before v2 goes GA. |
jldec
changed the title
|
@LorisSigrist @martin-lysk cc: @felixhaeberle (mock copied from #2793) |
|
Wow wrong button during scrolling on mobile - sorry for the ping. I am already afk - I can have a Quick Look at the ast tomorrow. Reagarding the discussion: If we want an interims string based api - we should use current state of icu2. If not it’s an object based typed api - that was at least the two path we discussed (?) @samuelstroschein ? |
Yes. I wouldn't cook our own serialized format and I am hesitant to introduce unstable icu2 now. Hence, delay all those utility functions until either or? :
|
|
ok, parsePattern is gone - I'll get by without the feature for now. |
| type: "expression", | ||
| arg: { | ||
| type: "variable", | ||
| name: "count: plural", |
There was a problem hiding this comment.
I don't think this expression is what you meant, this literally reads as "a variable called count: plural"
I think you wanted to apply the plural function to the variable count.
The way to express this is by adding a function annotation to the expression:
{
type : "expression",
arg: { type : "variable", name: "count"},
annotation: { type: "function", name: "plural", options: [] }
}
There was a problem hiding this comment.
That was my fault 😅 I naively created a mock message (which @jldec copied) without respecting the correct structure.
| pattern: [ | ||
| { | ||
| type: "text", | ||
| value: "Show {count} message.", |
There was a problem hiding this comment.
The value: "Show {count} message." doesn't treat "{count}" as a variable, but as plain text.
The way to mix in variables into the AST is by adding expressions to the pattern.
pattern: [
{ type: "text", value: "Show "},
{ type: "expression", arg: { type: "variable", name: "count" }},
{ type: "text", value: " message."}
]
|
I'll take myself out of the review loop here. Nothing to add besides @samuelstroschein & @LorisSigrist already valuable comments. |
|
I am also taking myself out of this discussion now. @LorisSigrist @jldec @martin-lysk you'll make the right choice |
|
Thanks for the feedback and suggestions. @felixhaeberle, @LorisSigrist, @martin-lysk please feel free to add additional examples using a similar directory naming pattern. |
This is an alternative implementation of #2773 (closed) which used MF2 string format internally.
Resolves opral/inlang-sdk#64
NOTE: Input variable declarations and variables in patterns are now wrapped inside an Expression object which adds more nesting to the ast structure. (see test for example)
Example