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

How cool is this package! #28

Open
Tracked by #1412
amannn opened this issue Nov 4, 2024 · 3 comments
Open
Tracked by #1412

How cool is this package! #28

amannn opened this issue Nov 4, 2024 · 3 comments

Comments

@amannn
Copy link

amannn commented Nov 4, 2024

Oh my god, how cool is this package? I made a note of this some time ago after stumbling upon this, but finally got the time to check out the ICU parser in types implementation in detail. Mind. blown. 🤯

For some context, I maintain an i18n library for Next.js called next-intl. I think it has quite some similarities with your package, also being based on the Format.js parser. Currently, the library provides type-safe message keys/namespaces, but ICU args aren't extracted so far. I finally have some time for this, that's why I landed here again.

I prototyped a quick integration based on your implementation here, and I think this works absolutely fantastic. I was quite impressed when I saw that {now, date} extracts the arg as a date value, but when I saw that you even implemented a union type for select args, I think that's when I lost it 😄

I was wondering if you're in any way interested in collaborating? I'd really like to give you credit for the implementation. Would you be interested in publishing the ICU parser as a standalone package that could be consumed by libraries that only need the types?

I think there'd be a few details about the API that we'd have to define, e.g. next-intl doesn't support Temporal yet, so e.g. injecting the types for ICU values could be something. Extracting tags like <b>Hello</b> could be another one (that one's easy). I think the relevant part of the ICU parser for next-intl would be around 160 LoC.

Do you think it would generally make sense to share the ICU parser type implementation? Or do you think it's easier if I just fork and adapt the relevant parts?

@schummar
Copy link
Owner

schummar commented Nov 4, 2024

Hi, glad you like it 😉. Note that there might be an edge case with escaping # in plurals not covered right now. See the discussion in #23.

Sure, I can publish the ICU parser as a separate package. I already did some preparation, but it probably needs some more cleanup and testing. Providing the supported ICU Values is also not a problem, I just needs to pass a config object down. I expect I can finish this in the next days.

Regarding extraction of tags - I would need to investigate this some more, because I simply ignored those when I built this library. What would you expect to the extracted there? E.g. what should the result of GetICUArgs<'<b>Hello</b>'> be?

@amannn
Copy link
Author

amannn commented Nov 5, 2024

That sounds great!

I've cleaned up a few things on my end, and I think something like this could be an API that would be consumable:

type ICUArgs<
  T extends string,
  ICUArgument,
  ICUNumberArgument,
  ICUDateArgument
> = 

(example)

I'm currently passing in type arguments for ICUArgument, ICUNumberArgument and ICUDateArgument—that might provide sufficient flexibility for my case. Maybe there's an easier way to pass all types around, you mentioned a "config object"?

I've also removed any supplemental types for typing the t function (or related features like ProvidedArgs) as my library already provides this as necessary on its own.

Would something like this work for schummar-translate too?

Regarding tag extraction: I've currently implemented this separately via ICUTags.tsx and merged extracted arguments as well as tags. So if you prefer to leave that out, that'd be fine too!

But to answer your question:

E.g. what should the result of GetICUArgs<'Hello'> be?

In next-intl there are in fact two cases where a different return value is expected:

  1. t.rich expects (chunks: ReactNode) => ReactNode
  2. t.markup expects (chunks: string) => string

So in the case of '<b>Hello</b>', there should be a tag named b extracted that receives a provided type.

So if you'd like to include tag extraction in your library, ideally the consumer could provide a type argument for this too.

Does that make sense to you?

@schummar
Copy link
Owner

Ok, I have published a separate type parser library now: https://www.npmjs.com/package/@schummar/icu-type-parser
I spent most of the time fighting the tooling 😬

Passing in the types work like this:

import { GetICUArgs } from '@schummar/icu-type-parser';

interface Options {
  ICUNumberArgument: 'mynumbertype';
  ICUDateArgument: 'mydatetype';
  ICUArgument: 'mydefaulttype';
}

type Result = GetICUArgs<'some string with {var}', Options>;

Demo: https://stackblitz.com/edit/vitejs-vite-ijpy9g?file=index.ts

Regarding tag extraction: I've currently implemented this separately via ICUTags.tsx and merged extracted arguments as well as tags. So if you prefer to leave that out, that'd be fine too!

I'll definitely look into this. It would be nice to have for my lib as well. Not sure when, but also contributions are welcome 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants