-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
[Bug]: JSON files isn't available to downstream ts_library type checking #516
Comments
Yeah I think technically this is a typescript tsc issue, in that it doesn't copy the |
Ah, first of all it took me a while to repro within rules_ts because we set I see 12012a4 actually added the explicit .json outputs, so my previous comment was wrong. It looks like that PR put the .json outputs into the "js" output group, rather than the "types". |
I have spent a few hours digging into this, and I'm still not sure it's possible. TypeScript handles It's easy to repro by cloning the @gzm0 does this make sense to you? Any ideas? Detailed analysis
(note, we have to patch this after the "Add JS inputs that collide with outputs" occurs, since data.json -> data.json couldn't be pre-declared)
This error doesn't happen for other files we got from Possible to workaround?Assuming no fix in |
I think the fix we want to argue for (and likely send a PR) is that the "special case" mentioned in microsoft/TypeScript#38015 (comment) should be extended. There are two ways I'd argue this:
@pyrocat101 I suspect we can patch this second behavior into your |
TY for pinging me on this @alexeagle. We have run into this issue as well. What works as a workaround for us, is to assign the JSON into an intermediate variable:
We are using this for the JSON values but it seems to work as well for types (verified on the i516 branch). What is happening is that when compiling -import data from './data.json';
+declare const data: {
+ a: string;
+}[];
export declare const a: string;
export declare type Data = typeof data;
+export {}; IDK why the I'll have a closer look at the references @alexeagle posted. This does feel like a tsc issue to me, I'll attempt to reproduce this issue without bazel (just One thing that stands out for me in all the repros I've seen (not the one we have internally) is that bazel |
I can reproduce this issue without bazel:
It seems this is essentially an occurrence of microsoft/TypeScript#25636 The workaround seems to be to simply configure all json files in
IIUC the situation correctly, IMHO the proper fix would be that typescript does not rely on JSON imports for types in its generated |
Thanks @gzm0 - that workaround has the right shape IMO. Getting the type signature inlined (like --isolatedDeclarations proposal?) does improve incrementality too - if you change values in |
You mean microsoft/TypeScript#47947 ? IIUC this would be huge for bazel, since we could avoid forwarding all declaration files of the dependencies. |
Ah thanks. @gzm0's workaround works well enough for my use case. |
What happened?
When
resolveJsonModule
is set, TypeScript type check will read JSON files to infer the types. For example:Type checking
//bar
will throw an error thatdata.json
cannot be resolved. The file is not present in the runfiles because only.d.ts
files offoo
are present.Version
Development (host) and target OS/architectures:
Output of
bazel --version
: aspect 5.8.19Version of the Aspect rules, or other relevant rules from your
WORKSPACE
orMODULE.bazel
file: rules_ts 2.1.0, rules_js 1.34.0Language(s) and/or frameworks involved: TypeScript
How to reproduce
Minimal repro: https://github.com/pyrocat101/ts-json-repro
Any other information?
No response
The text was updated successfully, but these errors were encountered: