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

typings are incomplete for useTranslation #766

Closed
pixelass opened this issue Jul 24, 2020 · 11 comments
Closed

typings are incomplete for useTranslation #766

pixelass opened this issue Jul 24, 2020 · 11 comments

Comments

@pixelass
Copy link

Describe the bug

The exported useTranslation is not typed appropriately.

TS2339: Property 'localeSubpaths' does not exist on type 'InitOptions'.

Occurs in next-i18next version

"next-i18next": "^4.5.0"

Steps to reproduce

const LocaleLink = ({ as, href, ...props }) => {
	const {
		i18n: {
			language,
			options: { localeSubpaths }, // TS2339: Property 'localeSubpaths' does not exist on type 'InitOptions'.
		},
	} = useTranslation();
	const lang = Object.values(localeSubpaths).includes(language) ? language : "";
	const withAbsolute = href.substr(0, 1) === "/" ? "/" : "";
	return <Link {...props} href={pathJoin(withAbsolute, lang, href)} as={as} />;
};

Expected behaviour

As a developer I expect the typings to reflect the actual interfaces exposed by the methods/functions.

Screenshots

Screen Shot 2020-07-24 at 14 37 59

OS (please complete the following information)

  • Device: MacBook Pro (16-inch, 2019)
  • Browser: Chrome Version 84.0.4147.89 (Official Build) (64-bit)

Additional context

I think the useTranslation (re-export from react-i18next) should be exported with an extended type.

@isaachinman
Copy link
Contributor

Why are you trying to pass localeSubpaths into useTranslation?

@pixelass
Copy link
Author

I am trying to extract/use it. It is present in i18n.options returned by useTranslation but the typings don't reflect it. I noticed when adding a localized link as in #413 (comment)

@pixelass
Copy link
Author

via

const { i18n } = useTranslation();
console.log(i18n);

i18n-useTranslation

@isaachinman
Copy link
Contributor

@pixelass useTranslation comes directly from react-i18next. I believe it returns the i18n instance from the context provider (if available), so it'd make sense that you see it there. However, you definitely do not need to pass localeSubpaths or any other config options into it – they're already there, because next-i18next inits the instance for you.

If you're curious, feel free to just dive into the source code.

@pixelass
Copy link
Author

pixelass commented Jul 24, 2020

@isaachinman I looked through the source but it seems since localeSubpaths is injected by next-i18next it is not present and is not an issue with react-i18next.
I think you might have misunderstood since the error is reporting InitOptions. useTranslation actually returns this interface. To clarify: "I don't inject localeSubpaths but I rather use it from the return value"

Since localeSubpaths is not part of react-i18next it is not their task. Although if they allowed passing custom options to the types it could be wrapped by next-i18next

Example:

import {useTranslation as useOriginalTranslation} from `react-i18next`
export interface Options {
  localeSubpaths: Record<string, string>
}
export const useTranslation = useOriginalTranslation<Options>

@isaachinman
Copy link
Contributor

Why do you need to use/inspect the i18n instance which is output by useTranslation?

@pixelass
Copy link
Author

pixelass commented Jul 24, 2020

See: #413 (comment)

const lang = Object.values(localeSubpaths).includes(language) ? language : "";

It takes care of the issue mentioned here: #764

I could make the link as follows:
<Link href="/[lang]/product/[name]" as="/product/test"><a>Test</a></Link>
The problem with this is my default language (English: "en") does not have a language set in the URL.
en: /product/test
no: /no/product/test

...where not every locale is using subPaths.
Since the changes I proposed to Link need this support I need access to this property to prevent having to manually add it (reduce complexity).
The new version of Link would then be a minor instead of a major change or maybe even a patch since it's really just fixing a bug.

@pixelass
Copy link
Author

pixelass commented Jul 24, 2020

Maybe I can propose a change in the typing https://github.com/i18next/react-i18next/blob/master/src/index.d.ts#L60-L63

to allow adding some custom interface extension

export function useTranslation<O>( // O was added
  ns?: Namespace,
  options?: UseTranslationOptions, 
): UseTranslationResponse<O>; // O needs to be passed down

@pixelass
Copy link
Author

pixelass commented Jul 24, 2020

It might be more complicated since react-i18next is using the i18n type provided by i18next which does not allow adding arguments.

We would need it to allow this:

interface i18n<O = {}> {
 //...
options : {/*...*/} & O
}

@isaachinman
Copy link
Contributor

You don't need to look into the output of useTranslation, you can just look at the i18n instance directly. Also, inserting a value out of Object.values(localeSubpaths).includes(language) is not going to work.

@pixelass pixelass changed the title typings ar incomplete for useTranslation typings are incomplete for useTranslation Jul 24, 2020
@pixelass
Copy link
Author

pixelass commented Jul 24, 2020

You don't need to look into the output of useTranslation, you can just look at the i18n instance directly. Also, inserting a value out of Object.values(localeSubpaths).includes(language) is not going to work.

I don't have access to the instance since it is defined locally. https://github.com/isaachinman/next-i18next#3-project-setup
I am also not inserting anything but simply checking if the language is defined as subPath or not.

const NextI18Next = require('next-i18next').default

module.exports = new NextI18Next({
  defaultLanguage: 'en',
  otherLanguages: ['de']
})

I haven't tried to add it to the core but I think it will resolve once I start and take a detailed look at the core of this library.

Sorry, I've had a long day and my brain doesn't seem to function that well anymore :)

Just ignore it for now. I'll open a new (more precise) issue if I encounter any conflicts.

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