MESDK-77 Requirements for Gender and Plurals [WORKING DRAFT] #2655
Conversation
🦋 Changeset detectedLatest commit: d1879fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
|
@LorisSigrist |
…requirements-for-gender-and-plurals
🥷 Ninja i18n – 🛎️ Translations need to be updated❗️ New errors in setup of project
|
| displayName, | ||
| description, | ||
| settingsSchema: PluginSettings, | ||
| loadMessages: async ({ settings, nodeishFs }) => { |
There was a problem hiding this comment.
we may consider adding an experimental function "importMessages" that uses the a Message2 type that can be mapped to the MessageLegacy. By doing so we can bypass the compatibility problems we face at the moment - just thinking out loud will discuss with you and @jldec tomorrow
| export type Declaration = Static<typeof Declaration> | ||
| export const Declaration = Type.Union([LocalDeclaration, InputDeclaration]) | ||
|
|
||
| export type Translation = Static<typeof Translation> |
There was a problem hiding this comment.
Translation vs. Locale - I think locales is the better term. It feels also more alinged with ICU glossary compare source-locale and locale.
Since this property is used everywhere (also in fixtures for testing) this naming should be chosen wisely TBD.
There was a problem hiding this comment.
Loris will doublecheck the glossary and we will collect reasons for the term here
| selectors: Type.Array(Expression), | ||
| variants: Type.Array(Variant), | ||
| }) | ||
|
|
||
| export type Message = Static<typeof Message> | ||
| export const Message = Type.Object({ |
There was a problem hiding this comment.
Lets discuss the pros / cons of a separate type for the internal representation to stay backward compatible with old plugins. We can prepare the discussion tomorrow.
There was a problem hiding this comment.
A new type makes sense -next steps tbd with @jldec
@jldec i hope we can reduce the friction we create with this pr a lot by introducing a separate type internally and map the in/out type we use in load/saveMessage to the newly created type (related related), we can discuss a possible strategy in person tomorrow. |
There was a problem hiding this comment.
great work @LorisSigrist - just smaller findings. Good starting point to iterate further.
@jldec I marked the places where we introduce the new AST format (naming not finalized). Create message is marked as deprecated - i will investigate together with @LorisSigrist how a first internal version - that will than be called by createMessage - would look like and what functions we need alternatives for.
inlang/source-code/sdk/src/legacy.ts
Outdated
| } | ||
| } | ||
|
|
||
| export function fromLegacyMessaeg(legacyMessage: LegecyFormat.Message): AST.MessageBundle { |
| selectors: [], | ||
| variants: [ | ||
| inputs: [], | ||
| translations: [ |
| /** | ||
| * @throws If the message cannot be represented in the legacy format | ||
| */ | ||
| export function toLegacyMessage(bundle: AST.MessageBundle): LegecyFormat.Message { |
There was a problem hiding this comment.
@jldec this is what i meant with a mapper from the new MessageBundle type to the legacy format - also see fromLegacyPattern for the other direction
| displayName, | ||
| description, | ||
| settingsSchema: PluginSettings, | ||
| importMessages: async ({ settings, nodeishFs }) => { |
There was a problem hiding this comment.
@jldec this a new function we use to keep the signature of loadMessages (soon to be deprecated) untouched
There was a problem hiding this comment.
ok - I wonder if there isn't a way to feature flag the api?
inlang/source-code/sdk/src/ast.ts
Outdated
| @@ -0,0 +1,120 @@ | |||
| import { LanguageTag as Locale } from "@inlang/language-tag" | |||
There was a problem hiding this comment.
@jldec Those types describe the messageBundle in which we represent the messages internally in.
There was a problem hiding this comment.
rename ast to message-bundle?
|
|
||
| export const createMessage = (id: string, patterns: Record<string, Pattern | string>) => | ||
| /** | ||
| * @deprecated Uses Legacy Message Format |
There was a problem hiding this comment.
@jldec here we use the LegacyFormat - we deprecate this function to allow fink / sherlock etc to adopt the new (to be defined functions) but stay backward compatible
There was a problem hiding this comment.
ok - I'm a bit worried about deprecation noise in our build logs - only use if really necessary
|
@martin-lysk The new interface package and importer plugin package both feel a bit premature for the scope of this PR - Are they really necessary? What capabilities are they expected to unlock for our app developers Why not ship a small sdk-integrated importer similar to the experimental persistence feature, just for testing. (The same is perhaps true also for the development-project in this PR - I think I'd prefer to maintain test projects for the sdk under the sdk tree) |
|
@jldec yes - agree: we will keep the final PR small. |
…ows with concrete implementation
| import * as MF from "messageformat" | ||
| import { displayName, description } from "../marketplace-manifest.json" | ||
| import { PluginSettings } from "./settings.js" | ||
| import { AST } from "@inlang/messages" |
There was a problem hiding this comment.
@martin-lysk what is this AST object? I can't find the @inlang/messages package
|
Thanks @martin-lysk I asked @LorisSigrist about the motivation for the importer and he confirmed that
So my preference would be to follow the pattern already shipping with the experimental persistence flag, and move that importer code inside the sdk for now. That would eliminate the complexity associated with building and loading a separate plugin while we are iterating on the persistence architecture. |
|
closing with branch intact. |
please don't delete this branch - parts are still being extracted thanks - @jldec
This PR updates the internal
MessageAST to include all the functionality we need to implement plurals, genders and other formatting functions.It also adds the experimental and not for public use
icu2-message-formatplugin which is able to load (not save) messages from icu2 files.CI will probably freak out because the Message-Type was heavily edited, but that's alright for the moment
HOW TO TEST
versioned-interfaces/messageplugin/icu2-message-formattestindevelopment-projects/icu2This should log the loaded messages. However, for some reason the SDK doesn't return any values or errors. The plugin does work (can also be seen in the logs).