unsplash / intlc

Compile ICU messages into code. Supports TypeScript and JSX. No runtime.
MIT License
56 stars 2 forks source link

Flattening doesn't traverse callbacks #199

Closed samhh closed 1 year ago

samhh commented 1 year ago

Given the message:

Experience the best of Unsplash with a <u>{months, plural, one {#-month} other {#-month}} foobar</u> from our friends at {company}.

We will fail to flatten on account of the plural being inside of a callback. (The output will be identical.)

We currently hoist all interpolation types to the top to achieve flattening. Callbacks are unique in that whilst they're interpolations of a sort, they act as functions on their contents, so can't be flattened in the same way.

There are two possible interpretations of this problem. The first is that we can aggressively hoist around callbacks as if they're not there, in which case we'd expect the following output:

{months, plural, one {Experience the best of Unsplash with a <u>#-month foobar from our friends at {company}</u>.} other {Experience the best of Unsplash with a <u>#-month foobar from our friends at {company}</u>.}}

The more defensive interpretation is that we can't do that, but we can at least hoist within a callback itself, in which case we'd expect:

Experience the best of Unsplash with a <u>{months, plural, one {#-month foobar} other {#-month foobar}}</u> from our friends at {company}.

I'm inclined to say we could safely take the aggressive approach.

I've never been terribly happy with our flattening algorithm. I don't know how easy this will be to solve within the current design, but it's just a single function so we can go wild with it. In any case, here's a rough starting point:

diff ```diff diff --git a/lib/Intlc/Compiler.hs b/lib/Intlc/Compiler.hs index e29d87b..f8126cc 100644 --- a/lib/Intlc/Compiler.hs +++ b/lib/Intlc/Compiler.hs @@ -59,6 +59,11 @@ flatten = go mempty ICU.SelectNamed n xs _ -> ICU.SelectNamed' n (mapSelectCase rec <$> xs) ICU.SelectWild n w _ -> ICU.SelectWild' n (rec w) ICU.SelectNamedWild n xs w _ -> ICU.SelectNamedWild' n (mapSelectCase rec <$> xs) (rec w) + -- Callbacks are a special case in which we have interpolations, + -- but we can't semantically hoist it to the top. Instead we want + -- to give any of its contents the opportunity to hoist, presreving + -- the callback's position. + ICU.Callback n x _ -> error "unimplemented" _ -> go (prev <> curr) next -- Expands any plural with a rule to contain every rule. This makes ICU plural diff --git a/test/Intlc/CompilerSpec.hs b/test/Intlc/CompilerSpec.hs index be2632e..65c8b05 100644 --- a/test/Intlc/CompilerSpec.hs +++ b/test/Intlc/CompilerSpec.hs @@ -133,6 +133,61 @@ spec = describe "compiler" $ do flatten x `shouldBe` y + it "flattens callbacks" $ do + let x = mconcat + [ "Today is " + , Callback' "bold" (mconcat + [ ">" + , SelectNamedWild' + "day" + (fromList + [ ("Saturday", "the weekend") + , ("Sunday", "the weekend, barely") + ] + ) + "a weekday" + , "<" + ] + ) + , "." + ] + + let y = + SelectNamedWild' + "day" + (fromList + [ ("Saturday", (mconcat + [ "Today is " + , Callback' "bold" (mconcat + [ ">the weekend<" + ] + ) + , "." + ] + )) + , ("Sunday", (mconcat + [ "Today is " + , Callback' "bold" (mconcat + [ ">the weekend, barely<" + ] + ) + , "." + ] + )) + ] + ) + (mconcat + [ "Today is " + , Callback' "bold" (mconcat + [ ">a weekday<" + ] + ) + , "." + ] + ) + + flatten x `shouldBe` y + describe "expanding rules" $ do let f = expandRules ```

For reference, here's formatjs' implementation: https://github.com/formatjs/formatjs/blob/b5976d9c75e8811470a26c286776cd82ec900b46/packages/icu-messageformat-parser/manipulator.ts#L35