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

Type safe message access #90

Closed
dnbkr opened this issue Mar 10, 2022 · 5 comments · Fixed by #93
Closed

Type safe message access #90

dnbkr opened this issue Mar 10, 2022 · 5 comments · Fixed by #93
Labels
enhancement New feature or request

Comments

@dnbkr
Copy link

dnbkr commented Mar 10, 2022

Describe the solution you'd like
I would find it very helpful if message access was type safe, for example:

given messages like this:

# messages/en.json

{
  "Components": {
    "Greeting": {
      "body": "hello, world!"
    }
  }
}

it would be good to have typescript fail on these two examples:

const t = useTranslations("Components.Foo")
const t = useTranslations("Components.Greeting")
t("bar")

Additional context
I appreciate this could be hard for splitting message sets across different pages (as you may not know which message set is available at any given time) so having all return types be possibly undefined could be a compromise. The aim of the feature is not so much to guarantee access safety but to facilitate auto completion of keys to avoid typos etc.

@dnbkr dnbkr added the enhancement New feature or request label Mar 10, 2022
@amannn
Copy link
Owner

amannn commented Mar 10, 2022

Thank you for opening this issue, I'd be interested in this too as it will enhance the DX quite a bit!

I'm not sure yet what's the best way to tackle this. I can think of the following approaches:

  1. Providing a generic parameter to every call site. This is implementation-wise probably the easiest, but the ergonomics are not really good.
useTranslations<Messages>()
  1. Providing a factory function to provide the generic parameter. This would allow to provide the parameter only once, but it changes the way you import every module – so also not ideal IMO.
const {useTranslations,} = createTypedIntlModules<Messages>();
  1. Use the ability from TypeScript to override an interface (e.g. like Emotion does it).

Not sure if there are other options, but from these I'd largely prefer the third one the user doesn't have to change any call site of the provided modules.

Another part is typing the t() function. I'm wondering if there's a way we can resolve the generic namespace in the useTranslations function and generate a more specific type for the t() function in return.

The other aspect is how to create the type. I'd guess there are some utilities from TypeScript to generate this, I'd image something like typeof to work here. This would have to be done in user land since we don't make assumptions about how or where the messages are stored. So this would largely be a documentation topic.

If you have other ideas or are interested in helping out to implement this please let me know!

@amannn amannn mentioned this issue Mar 18, 2022
5 tasks
@amannn
Copy link
Owner

amannn commented Mar 29, 2022

Hey @coffeedoughnuts,

I found some time to look into this topic and I think I've found a solution that works!

Would you like to try out a prerelease version of this feature in your app? I'd be really happy about some feedback if you have the time.

Here's how to try the prerelease:

npm install use-intl@2.4.1-alpha.5 next-intl@2.4.1-alpha.5

… and here are the docs for the new feature: https://next-intl-git-feat-90-type-safe-messages-amann.vercel.app/docs/usage/typescript.

/cc @ishigami since you accidentally found an earlier prerelease of this feature in #94 😁.

Also /cc @andrevenancio since you already provided helpful feedback in the past! Maybe you're curious 🙂

@dnbkr
Copy link
Author

dnbkr commented Mar 29, 2022

Hey - I'm so sorry, I had some time off from work and completely forgot to reply to you after I returned!

I shall try your solution later today and report back; thank you for looking into it!

@dnbkr
Copy link
Author

dnbkr commented Mar 29, 2022

small point, in case anyone else is trying this alpha, I could only get it working if I specifically installed next-intl@2.4.1-alpha.4 and use-intl!2.4.1-alpha.4 - npm would not install the alpha of the use-intl dependency until I told it to explicitly.

as for the implementation, it's great! as this package is quite closely related to next I would suggest in the documentation to perhaps not use a global ts declaration as out-of-the-box next doesn't include one and instead namespaces their own. A suggestion:

To do this, add a global type definition file in your project root (e.g. global.d.ts):

in the linked documentation, becomes:

To do this, add a type definition file at the root of your project called next-intl-env.d.ts. It should be alongside the generated next-env.d.ts. Paste the following into the new file:

type Messages = typeof import('./path/to/sample/messages.json');
declare interface StrictIntlMessages extends Messages {}

and add the file to the include array in your tsconfig.json:

"include": ["next-env.d.ts", "next-intl-env.d.ts", "**/*.ts", "**/*.tsx"],

it's a pretty minor change but it follows's next's convention for solving a similar situation

@amannn
Copy link
Owner

amannn commented Mar 31, 2022

Thank you for trying out the pre-release and your feedback! 🙌

After another thought I've renamed StrictIntlMessages now to just IntlMessages in 2.4.1-alpha.5, I think that's sufficient.

Hmm, about the name of the suggested file: I think Next.js has to use their own file, since they automatically write to this file and therefore they need a separate one. As for the the file that is currently suggested in the docs, this is just a snippet that can live alongside other library definitions. At least personally I wouldn't want to have a separate file per library that I'm declaring types for, especially if they only contain a few lines each.

But I'm not really sure what the best practice is here for TypeScript projects.

Here are some links I found:

There's no clear suggestion here though as far as I can tell. In projects I've worked on so far I've seen index.d.ts and global.d.ts.

I tried to not really take a stance here with this phrasing:

To do this, add a global type definition file in your project root (e.g. global.d.ts):

I wouldn't want the user to have to create a separate library definition for this library, therefore I think we should stick to a less commanding suggestion. I'd be really curious though if there is an established best practice for these types of files.

@amannn amannn closed this as completed in #93 Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants