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

tsc: OOM when using large number of arguments #695

Open
ridem opened this issue Jul 17, 2023 · 2 comments
Open

tsc: OOM when using large number of arguments #695

ridem opened this issue Jul 17, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@ridem
Copy link

ridem commented Jul 17, 2023

Version

5.24.2

Describe the bug

Note: Happy to submit a PR for this issue

The utility type RequiredParams, with its use of Permutation, makes our Typescript server crash due to OOM issues.

RequiredParams is fine as long as the number of arguments is OK, but when we've introduced a multiline string with 8 arguments in it, the number of permutations exploded to 8!=40320, and tsc crashed because it ran out of memory.

Here is the diagnostics information from our i18n package, without any external dependency.

tsc --diagnostics:

Files:              1616
Lines:            728468
Identifiers:      790619
Symbols:         5999545
Types:            516404
Instantiations:  2691792
Memory used:    2907860K
I/O read:          0.08s
I/O write:         0.00s
Parse time:        1.38s
Bind time:         0.61s
Check time:        6.05s
Emit time:         0.00s
Total time:        8.04s

tsc --diagnostics, with RequiredParams<...> | string replaced by string:

Files:             1616
Lines:           728468
Identifiers:     790311
Symbols:         447275
Types:             5864
Instantiations:    7532
Memory used:    672460K
I/O read:         0.08s
I/O write:        0.00s
Parse time:       1.47s
Bind time:        0.59s
Check time:       0.12s
Emit time:        0.00s
Total time:       2.18s

One way to solve it is to add a config to allow disabling the use of RequiredParams. The code to switch it on/off is already there, but basically depends on the TS version.
For now we've just patched the generator code to always spit out "string" instead of generateTranslationType(args) and this suits us.

Reproduction

On any sample project:

  1. Start the generator: npm run typesafe-i18n
  2. Run tsc --diagnostics. Write down the Symbols, Types, Instantiations, Memory used
  3. Add translation strings with a large number of arguments (8+). For example, a multi-line string.
  4. Run tsc --diagnostics again. Notice how the 4 values have exploded

Logs

No response

Config

{
  "$schema": "https://unpkg.com/typesafe-i18n@5.24.4/schema/typesafe-i18n.json",
  "outputPath": "./src/",
  "generateOnlyTypes": true
}

Additional information

No response

@ridem ridem added the bug Something isn't working label Jul 17, 2023
@ivanhofer
Copy link
Owner

Hi @ridem, I know about the problem and was waiting for someone to file an issue to see if it is actually a problem that occurs in practice.

I think the most convenient way to improve this is to output a type without a permutation in cases where nrOfParams > 7.
This needs to be done here:

const getParamsType = (argStrings: string[]) =>

Instead of a primitive string I would go for a type that restricts the parameter names, but allows any arbitrary combination of those:

const message1: T<'a', 'b'>` = 'hello {a} and {b}' // correct
const message2: T<'a', 'b'>` = 'hello {b} and {b}' // correct
const message3: T<'a', 'b'>` = 'hello {a} and {a}' // correct
const message3: T<'a', 'b'>` = 'hello {c} and {d}' // incorrect

This would make sure to offer the best possible typesafety, without running into the OOM issue.

Also this line needs to change to import the correct helper type:

shouldImportRequiredParamsType ? ', RequiredParams' : ''

It would be great if you could contribute a PR :)

@chriscoomber
Copy link
Contributor

For those who can't wait for this to be fixed in this branch (RIP Ivan, thanks so much for all your contributions), you can use patch-package to work around this.

I didn't care at all about type safety when providing translations in other languages, so I just patched out that whole feature. You may be able to find a more nuanced fix, like what Ivan suggested above. But my patch was simply:

patches/typesafe-i18n+5.17.0.patch

diff --git a/node_modules/typesafe-i18n/types/runtime/src/core.d.mts b/node_modules/typesafe-i18n/types/runtime/src/core.d.mts
index b8c7f07..3128efa 100644
--- a/node_modules/typesafe-i18n/types/runtime/src/core.d.mts
+++ b/node_modules/typesafe-i18n/types/runtime/src/core.d.mts
@@ -48,7 +48,9 @@ export interface ImportLocaleMapping extends LocaleMappingBase {
 type Permutation<T extends string, U extends string = T> = [T] extends [never] ? Array<string> : T extends U ? [T, ...Permutation<Exclude<U, T>>] : [T];
 type WrapParam<P> = P extends string ? `{${P}}` : never;
 type ConstructString<Params extends unknown[]> = Params extends [] ? `${string}` : Params extends [infer Param, ...infer Rest] ? `${string}${WrapParam<Param>}${ConstructString<Rest>}` : Params extends string ? Params : `${string}`;
-export type RequiredParams<Params extends string> = ConstructString<Permutation<Params>>;
+// patch-package START
+export type RequiredParams<Params extends string> = string;  // Was `= ConstructString<Permutation<Params>>;`
+// patch-package END
 export declare const isPluralPart: (part: Part) => part is PluralPart;
 export declare const translate: (textParts: Part[], pluralRules: Intl.PluralRules, formatters: BaseFormatters, args: Arguments) => LocalizedString;
 type GetArg<Arg extends string, Type> = Arg extends '' ? void : Arg extends `${infer OptionalArg}?` ? Partial<Record<OptionalArg, Type | undefined>> : Record<Arg, Type>;

i.e. I just changed RequiredParams to always simply be string. See the link above for patch-package for instructions to make your own patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants