Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select/boolean and unions #102

Open
OliverJAsh opened this issue Mar 29, 2022 · 7 comments
Open

select/boolean and unions #102

OliverJAsh opened this issue Mar 29, 2022 · 7 comments

Comments

@OliverJAsh
Copy link
Member

OliverJAsh commented Mar 29, 2022

Feel free to rename this issue. I couldn't think of a better name!

Example 1

Extracted from https://crewlabs.slack.com/archives/C0STWEZ2B/p1646839146215689

We have an optional value, topicTitleOption of type Option<string>. To use it conditionally inside of the ICU message we have to use the select/boolean syntax:

Your photos are now being reviewed {hasTopicTitle, boolean, true {and may be shared in the {topicTitle} topic} false {by the community and will soon be shared under the Unsplash license}}.

However, hasTopicTitle is required even when hasTopic is false (in which case topicTitle won't actually be used inside of the message).

t.successMessage({
  hasTopicTitle: pipe(topicTitleOption, O.match(constant(false), constant(false))),
  topicTitle: pipe(topicTitleOption, O.getOrElseValue('')),
})

topicTitle should only be required when hasTopicTitle is true, e.g. perhaps we could generate something like this:

export const foo = (
    x:
        | { hasTopicTitle: true; topicTitle: string; }
        | { hasTopicTitle: false; }
) => "…";

Alternatively, we could invent some new syntax to express an optional value:

now being reviewed {topicTitle, ifDefined, true {in the {topicTitle}} false {by the community}}

However I’m not sure how far we can go with that approach given that we need translators to be familiar with the syntax.

Example 2

Note: this example could be split into two separate messages instead of using select, but that's not always feasible i.e. if the select is surrounded by other text.

{shareType, select, email {Share over email} service {Share on {serviceName}}}

We have to pass in a dummy value for serviceName when shareType is email.

t.linkTitle({
  shareType: pipe(
    type,
    ButtonShareType.match({
      email: constant('email'),
      facebook: constant('service'),
      twitter: constant('service'),
      pinterest: constant('service'),
    }),
  ),
  serviceName: pipe(
    type,
    ButtonShareType.match({
      // Note: this is an impossible state
      email: constant(Intlzd.wrap('')),
      facebook: constant(Intlzd.wrap('Facebook')),
      twitter: constant(Intlzd.wrap('Twitter')),
      pinterest: constant(Intlzd.wrap('Pinterest')),
    }),
  ),
})

serviceName should only be required when shareType is service, e.g. perhaps we could generate something like this:

export const foo = (
    x:
        | { shareType: "email" }
        | { shareType: "service"; serviceName: string }
) => "…";
@samhh
Copy link
Contributor

samhh commented Mar 30, 2022

Before I forget my idea was that potentially output could look something like:

// Now
{ x: string; y: number }

// After
{ x: string } & { y: number }

This continues to offload everything to tsc and in theory makes it easy to branch at any point with a union:

{ alwaysProp: string } & ({ type: 'email' } | { type: 'service'; serviceName: string })

@OliverJAsh
Copy link
Member Author

Perhaps we could add a special case to select e.g. *null*:

now being reviewed {topicTitle, select, *null* {by the community} other {in the {topicTitle}}}

In the generated TS, the param topicTitle would have type string | null rather than just string.

@samhh
Copy link
Contributor

samhh commented Apr 6, 2022

That's interesting. I suppose it'd be analagous to other. Where other widens e.g. 'a' | 'b' to string (encoded conveniently via a union), null could widen/unionise to null. Not just in select, but also plural.

Ideally though a syntactic construct for null could apply to any type, otherwise we can't handle cases like Date | null. I think that's where the complexity I'd previously considered comes into play, it incorporates a sort of polymorphism (A | null).

If we say hypothetically the implementation would be doable, how about this?:

now being reviewed {topicTitle, nullable, null {by the community} other {in the {topicTitle}}}

# compiles `topicTitle` to `string | null`

Or a motivating example for the broader nullable type:

You joined {joined, nullable, null {a while ago} other {on {joined, date, long}}}

# compiles `date` to `Date | null`

@OliverJAsh
Copy link
Member Author

Interesting. I guess this means further deviating from the ICU spec, but otherwise I think this looks good! With regards to the ICU spec, do we need to be concerned about confusing translators with syntax they haven't seen before? Or perhaps this isn't an issue because I don't think the translators (in our case) deal with the raw ICU—they have a tool that parses the ICU message and displays it in a special UI.

@OliverJAsh
Copy link
Member Author

Thinking about this more, I think there are two separate problems described in the OP, each requiring their own solution. Should we split this up?

@samhh
Copy link
Contributor

samhh commented Apr 6, 2022

I agree, nullability warrants its own issue: #105

I've also raised this... #106

@samhh
Copy link
Contributor

samhh commented May 1, 2022

This'd break virtually all the TS tests, but here's roughly what a passing test with this feature would look like:

diff --git a/test/Intlc/EndToEndSpec.hs b/test/Intlc/EndToEndSpec.hs
index 4bc1c57..b443008 100644
--- a/test/Intlc/EndToEndSpec.hs
+++ b/test/Intlc/EndToEndSpec.hs
@@ -72,6 +72,10 @@ spec = describe "end-to-end" $ do
     [r|{ "f": { "message": "{x, select, a {hi} b {yo} other {ciao}}", "backend": "ts" } }|]
       =*= "export const f: (x: { x: string }) => string = x => `${(() => { switch (x.x) { case 'a': return `hi`; case 'b': return `yo`; default: return `ciao`; } })()}`"
 
+  it "deeply intersects and unionises arguments" $ do
+    [r|{ "f": { "message": "{name}'s preference: {type, select, email {email} service {{protocol, select, ftp {FTP} http {{isSecure, boolean, true {HTTPS} false {HTTP}}}}}}", "backend": "ts" } }|]
+      =*= "export const f: (x: { name: string } & ({ type: 'email' } | ({ type: 'service' } & ({ protocol: 'ftp' } | ({ protocol: 'http' } & { isSecure: boolean }))))) => string = `${x.name}'s preference: ${(() => { switch (x.type) { case 'email': return `email`; case 'service': return `${(() => { switch (x.protocol) { case 'ftp': return `FTP`; case 'http': return `${(() => { switch (x.isSecure) { case true: return `HTTPS`; case false: return `HTTP`; } })()}`; } })()}`; } })()}`"
+
   it "compiles selectordinal" $ do
     [r|{ "f": { "message": "{x, selectordinal, one {foo} other {bar}}", "backend": "ts" } }|]
       =*= "export const f: (x: { x: number }) => string = x => `${(() => { switch (new Intl.PluralRules('en-US', { type: 'ordinal' }).select(x.x)) { case 'one': return `foo`; default: return `bar`; } })()}`"

Maybe the type signature degrades a bit too much to take this approach?

export declare const f: (x: { name: string } & ({ type: 'email' } | ({ type: 'service' } & ({ protocol: 'ftp' } | ({ protocol: 'http' } & { isSecure: boolean }))))) => string

We could perhaps merge a little bit more (see at the end) at the cost of some complexity, but... it's not tons better.

export declare const f: (x: { name: string } & ({ type: 'email' } | ({ type: 'service' } & ({ protocol: 'ftp' } | ({ protocol: 'http'; isSecure: boolean }))))) => string

@samhh samhh removed the discussion label Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants