Skip to content

Conversation

ahrjarrett
Copy link
Member

@ahrjarrett ahrjarrett commented Jun 28, 2025

Closes #1758

This PR accompanies i18next/#2322.

What this PR changes

To keep the review of this PR simple, I figured it would help if I talked about the changes, since the actual change is quite small.

Here's a simplified version of what this PR does:

// Before
type TransProps<Key extends ParseKeys<Ns, TOpt, KPrefix>> = { 
    i18nKey?: Key | Key[];
}

// After:
type TransProps<Key> = { 
    i18nKey?: Key;
}

type GetTransProps<
    Key extends ParseKeys<Ns, TOpt, KPrefix>
> = TransProps<
        // there is one additional branch here, to decide 
        // what to pass as the translation source:
        _EnableSelector extends true
            ? SelectorFn
            : Key | Key[]
    >
>

Essentially all this PR does is adds an inline check for whether enableSelector is turned on; if so, it processes the tree of translations, rather than generation a union of strings.

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@ahrjarrett ahrjarrett marked this pull request as ready for review June 28, 2025 19:52
@coveralls
Copy link

coveralls commented Jun 28, 2025

Coverage Status

coverage: 95.864%. remained the same
when pulling 0330767 on ahrjarrett:selector
into da4d5e2 on i18next:master.

@adrai
Copy link
Member

adrai commented Jun 28, 2025

Decide: how should we handle "fallback" selections, as outline in this comment?

Currently the Trans componant has this defaultValue: const defaultValue = defaults || nodeAsString || reactI18nextOptions.transEmptyNodeValue || i18nKey;

@marcalexiei marcalexiei removed their assignment Jun 29, 2025
@marcalexiei
Copy link
Contributor

Like I mentioned earlier, I don't have time to look at this right now—feel free to go ahead without me.

@adrai
Copy link
Member

adrai commented Jun 29, 2025

@VIKTORVAV99 can you also have a look at this?

Copy link

stale bot commented Jul 19, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 19, 2025
@adrai adrai removed the stale label Jul 19, 2025
Copy link

stale bot commented Jul 29, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 29, 2025
@adrai adrai removed the stale label Jul 29, 2025
@ahrjarrett ahrjarrett requested a review from marcalexiei August 20, 2025 12:45
@adrai
Copy link
Member

adrai commented Aug 20, 2025

@ahrjarrett can you update the i18next version to 25.4.0 and test again if still all ok?

@adrai adrai merged commit 78aec36 into i18next:master Aug 20, 2025
7 of 8 checks passed
@adrai
Copy link
Member

adrai commented Aug 20, 2025

v15.7.0 has just been released

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

Successfully merging this pull request may close these issues.

Type instantiation is excessively deep and possibly infinite.ts(2589)
4 participants