-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Import JSON in Typescript #25516
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
Import JSON in Typescript #25516
Conversation
|
|
||
| const tsConfigPath = resolve(projectFlag); | ||
| return PROJECTS.filter(project => project.tsConfigPath === tsConfigPath); | ||
| return PROJECTS.filter(project => project.tsConfigPath.includes(tsConfigPath)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it useful to be able to run tslint for x-pack like this:
node scripts/tslint --project x-pack
Is that okay, or should I revert this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be fine with something a little bit stricter, maybe:
project.tsConfigPath === tsConfigPath || project.tsConfigPath === resolve(tsConfigPath, 'tsconfig.json')Pretty sure as is --project . will match any tsconfig.json file, depending on which one is first.
| "test/**/*" | ||
| ], | ||
| "compilerOptions": { | ||
| "resolveJsonModule": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially added this to kibana/tsconfig.json but it caused problems in kbn-pm/tsconfig.json and kibana/tsconfig.browser.json since they do not emit commonjs. I can probably override this with "resolveJsonModule": false if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you wouldn't mind setting this in the root config and negating it where necessary that would keep the compilerOptions a little more consistent across projects, which is desirable.
|
The build is currently failing. I can repro the issue locally by runing @spalger or anyone else: any good ideas for fixing the above? |
💔 Build Failed |
|
@sqren that's a tricky one... We rely on the TypeScript being written in place right now and moving away from that will be pretty tricky to get working in the build process, but I also don't know of any way to force TS to write the output or to not emit .json files. I wonder if we could avoid using the actual CLI and take over emitting the results, skipping over the .json files? |
That sounds like a good approach, and I might do that in the future if no-one beats me to it. However, for now I'll close this issue. I'll outline the reason below. Workaround data.json [{ "value": 0 }, { "value": 1 }, { "value": "NaN" }]filterItems.ts import { data } from './data.json'; // will be of type: ({ value: number | string })[]
interface Item {
value: number | 'NaN';
}
function filterItems(items: Item[]) {
return items.filter(item => item.value !== 'NaN');
}
filterItems(data); // ERROR: Type 'string' is not assignable to type 'number | "NaN"'Above fails because typescript infers the list of values as That issue goes away when annotating with types: data.ts import { Item } from './Item'
export const data: Item[] = [{ value: 0 }, { value: 1 }, { value: 'NaN' }];filterItems.ts import { data } from './data.ts'; // will be of type: ({ value: number | 'NaN' })[]
interface Item {
value: number | 'NaN';
}
function filterItems(items: Item[]) {
return items.filter(item => item.value !== 'NaN');
}
filterItems(data); // No errors :)There are of course still advantages to being able to import json but for now it doesn't seem worth the hassle. |
💔 Build Failed |
Currently it is not possible to import json files in typescript. In APM we've worked around this by adding a declaration file, that declares json files as
any. This is sub-optimal, since we lose all the type info available in a static config.Enabling
resolveJsonModulewill let devs import .json as they .import other files. It only works whenmodule: "commonjs".