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

Support overriding the types of a set of JSON files #49703

Closed
5 tasks done
fpintos opened this issue Jun 28, 2022 · 9 comments
Closed
5 tasks done

Support overriding the types of a set of JSON files #49703

fpintos opened this issue Jun 28, 2022 · 9 comments
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@fpintos
Copy link
Member

fpintos commented Jun 28, 2022

Suggestion

πŸ” Search Terms

JSON declare module change type resolveJsonModule

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Allow applications to override/declare the imported type of a set of JSON modules.

We seem to be able to declare types of imported modules with declare module, but if the target module ends with .json extension, the declaration does not seem to take effect. For example, this works, with some limitations:

type ResourceId = string & { __ResourceId: void };
declare module "*.locstring" {
  const value: { [key: string]: ResourceId };
  export default value;
}

But this does not work (just because of the .json extension):

type ResourceId = string & { __ResourceId: void };
declare module "*.locstring.json" {
  const value: { [key: string]: ResourceId };
  export default value;
}

πŸ“ƒ Motivating Example

This feature would allow developers to, at compiler time, use branded string types to identify strings that were loaded from specific JSON files, as opposed to plain strings. The compiler could then detect if functions that expect such branded strings are called with other types of strings.

πŸ’» Use Cases

Our application stores strings for localization in *.locstring.json files.
For example, hello.locstring.json might contain:

{ "hello": "Hello World" }

Code consumes these strings by importing the locstring file and calling a loc(resourceId: string): string function. For example:

import React from 'react';
import strings from './hello.locstring.json';
export function Hello() {
    return <div>{loc(strings.hello)}</div>
}

We're using resolveJsonModule, so the strings variable is properly typed and the compiler will detect if you try to access a field that does not exist, like strings.helloWorld. The type of strings.hello comes as string as one would expect from a normal .json file.

This presents a problem - while it is valid to call loc with strings.hello as parameter, it should be invalid to call loc with "Hello World", for example. When multiple functions and parameters are involved, people accidentally end up calling things that look like loc(loc(strings.hello)) and that does not work, because loc expects the parameters to be 'resource ID' (ie, fields from the locstring files), and not any plain string.

The reason for this is that in debug mode, loc is pretty much a pass-through, so it outputs the "localized" string as found in the JSON file. But in retail there is a lookup involved. At build time, the contents of all *locstring.json files are processed and the string we get back from strings.hello ends up being a unique identifier, let's say "a2w" or something like that. When the application runs, it will load the actual set of strings and put them into a map, and then loc will take the given resourceId passed as parameter and lookup the corresponding string in the current language.

In this scenario, passing a random plain string to loc as a parameter is a bug, since it will not exist on the map - or worse, the random string is user input (like a file name) and it just happens to match one of the resource ID and it would be localized to a random string, so that can't happen.

Ideally, we want to detect invalid calls to loc at compile time.

We could use a branded string type to limit the types of strings one can pass to loc, for example:

type ResourceId = string & { __ResourceId: void };
export function loc(resourceId: ResourceId) : string {
   return resourceIdToLocalizedStringMap[resourceId];
}

Doing so causes every call to loc(string.someId) to be flagged as an error, because the compiler rightfully believes string cannot be assigned to ResourceId.

Trying to tell the compiler that the fields it gets from the *.locstring.json files are in fact ResourceId does not seem to work:

declare module "*.locstring.json" {
  const value: { [key: string]: ResourceId };
  export default value;
}

// We end up with this, suppose test.locstring.json has two strings in it, Hello and World:
import locStringJson from "./test.locstring.json";

// import from *.locstring.json does have both fields, but still typed as plain strings.
// I wish these would be typed as ResourceId
export const hello: string = locStringJson.Hello;
export const world: string = locStringJson.World;
//export const worldAsResourceId: ResourceId = locStringJson.World; // BAD: Fails with string cannot be assigned to ResourceId
//export const invalid: string = locStringJson.Bad; // GOOD: Correctly fails with 'Property Bad does not exist in type...

On the other hand, IF the files were named just .locstring (without the .json extension) then it kinda works, although we lose one thing in the process:

import locStringOnly from "./test.locstring";

// This is not ideal, since our strings are in *.locstring.json files, but it shows there could be some way to do it.
// import from *.locstring has proper type in fields, but since it comes up as a record, one can make typos.
export const hello2: ResourceId = locStringOnly.Hello2;
export const world2: ResourceId = locStringOnly.World2;
export const invalid: string = locStringOnly.Bad; // BAD: Ideally this must fail with 'Property Bad does not exist in type...

For us, renaming the files at this point is not feasible because there are several devops processes around them.

The other issue is that, IF the declare module "*.locstring.json" were to work as expected, with that particular definition we would be losing the detection of incorrect field names. That comes from writing simply value: { [key: string]: ResourceId } in the declaration. I'm not sure there's a way to refer to the original type of the JSON.

Bonus points:
Our locstring files can contain multiple strings, and every localizable string must also contain a comment for localization to know how to properly deal with it, with the name derived from the original name. For example, a login.locstring.json file would look like this:

{
    "nameLabel": "Name",
    "_nameLabel.comment": "Label for the username field in the login screen",
    "passwordLabel": "Password",
    "_passwordLabel.comment": "Label for the password field in the login screen",
}

Ideally only strings.nameLabel and strings.passwordLabel would be valid ResourceId to be passed to loc.
That being said, it is not that big of a deal if the "_id.comment" fields also come up as ResourceId because using them is cumbersome and people don't make that mistake.

So, the two requests in this issue are:

  • Can we allow declare module "*.some_extension.json" to override the type produced by a plain .json file?
  • Can we write the overriding type in such a way that it will preserve the set of fields the plain .json file would have had, replacing only the types of the fields?
@fpintos
Copy link
Member Author

fpintos commented Jun 28, 2022

A few extra comments:

In our codebase, we have several thousand locstring files, and that many more imports of them, so explicitly casting each import would be considered cumbersome.

@RyanCavanaugh
Copy link
Member

Can we allow declare module "*.some_extension.json" to override the type produced by a plain .json file?

I don't see any immediate blocker here. @weswigham any concerns with making ambient module declarations apply to json extensions? I would have expected this to work already

Can we write the overriding type in such a way that it will preserve the set of fields the plain .json file would have had, replacing only the types of the fields?

This would be a pretty big architectural lift from our end; I'd really rather not. This seems like a place where some pre-build tooling would be appropriate depending on the scenario.

@weswigham
Copy link
Member

any concerns with making ambient module declarations apply to json extensions? I would have expected this to work already

Yeah, I don't know why it doesn't already - probably some weird prioritization logic for json modules.

This seems like a place where some pre-build tooling would be appropriate depending on the scenario.

Definitely. It's what we do here - generate declarations from loc json.

It's probably worth noting that even if ambient pattern declarations don't currently apply to .json documents, you can write a .json.d.ts that we will pick up, at least in commonjs (I'm less sure about in modes that require exact extension matches...).

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jul 7, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 7, 2022
@RyanCavanaugh RyanCavanaugh added the Help Wanted You can do this label Jul 7, 2022
@muzuiget
Copy link

muzuiget commented Jul 18, 2022

TypeScript infers the JSON types, this behavior is really painful, because sometime JSON files are use as "database-like" code.

for example:

// config-like json
// hand writing JSON, it's good for tsc to infer the types.
{
    "username": "hello",
    "password": "world"
}
// database-like.json
// auto-generate JSON, it's bad for tsc to infer the types.
[
    {"a": 1},
    {"a": 2},
    {"a": 3},
    ...
    {"a": 10000},
]

// tsc will raise "JavaScript heap out of memory" if the JSON file is so big
import data from './database-like.json';

Iooking for something feature disable the JSON type infer:

// tsc should import as type `any` or `unknown`
import JSON_DATA from './database-like.json';

type Item = Record<string, any>;

const data = JSON_DATA as Item[]; // cast it for later use

@weswigham
Copy link
Member

weswigham commented Mar 15, 2023

JSON files can now have types provided for them by an adjacent .d.json.ts file - this is different than the details of the proposal in the OP's request, but definitely satisfies the title and is close enough that I'm OK closing this. A new issue can be filed in its' place explaining why the declaration file solution is somehow insufficient, should that be the case~

@holic
Copy link

holic commented Aug 9, 2024

JSON files can now have types provided for them by an adjacent .d.json.ts file - this is different than the details of the proposal in the OP's request, but definitely satisfies the title and is close enough that I'm OK closing this. A new issue can be filed in its' place explaining why the declaration file solution is somehow insufficient, should that be the case~

Sorry to dig up an old issue, but out of curiosity, is the intent of .json.d.ts files to also take precedence over the types from resolveJsonModule: true? Because when I am using both, I only ever get types from resolveJsonModule: true, never from my .json.d.ts files (essentially a copy of the JSON with as const at the end).

Happy to open an issue + repro for this to demonstrate.

@weswigham
Copy link
Member

You want .d.json.ts, not .json.d.ts. The first corresponds to a .json file, the later to a .json.js file, which, in the context of cjs automatically adding .js to your imports, was sometimes close enough, but is not quite the same.

@holic
Copy link

holic commented Aug 14, 2024

You want .d.json.ts, not .json.d.ts. The first corresponds to a .json file, the later to a .json.js file, which, in the context of cjs automatically adding .js to your imports, was sometimes close enough, but is not quite the same.

Thanks! I gave that a try, but TS couldn't seem to find the type declarations for the .json import like it usually would with .json.d.ts.

Module 'IWorldCall.abi.json' was resolved to 'IWorldCall.abi.d.json.ts', but '--allowArbitraryExtensions' is not set.

I enabled allowArbitraryExtensions and both TS and JS runtime are happy, but then tsup seems unable to build the DTS files for the project because it stumbles on not knowing what to do with .d.json.ts. I tried:

  • exclude in tsconfig.json to ignore **/*.d.json.ts files
  • specifying loader in tsup.config.ts to treat .d.json.ts as ts

To clarify, importing a .json file that had a corresponding .json.d.ts does seem to work when resolveJsonModule is disabled. So it seems like the tooling above mostly knows what to do with the declaration files with this naming, but not sure if that's a happy and unintended side effect of something else. And unclear why .json.d.ts files are sort of ignored once resolveJsonModule is enabled.

Anything else I should try? I feel like I am missing something.

@weswigham
Copy link
Member

but then tsup seems unable to build the DTS files for the project because it stumbles on not knowing what to do with .d.json.ts

I imagine tsup was never updated to handle allowArbitraryExtensions correctly, then, if it doesn't treat .d.*.ts files as declarations correctly (it shouldn't need to build a declaration file - it already is one) - I'd go file an issue on them and see what happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants