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

Doesn't work with bundled JSON #255

Closed
hoodmane opened this issue Aug 15, 2023 · 5 comments
Closed

Doesn't work with bundled JSON #255

hoodmane opened this issue Aug 15, 2023 · 5 comments

Comments

@hoodmane
Copy link

Bug report (or feature request?)

Source code

My project looks as follows:

a.json

{
    "a" : 2,
    "b": 7,
    "c": [1, 2, 3]
}

b.ts

import {a, b, c} from "./a.json";
export const x = {a, b, c};

tsconfig.json

{
  "compilerOptions": {
    "resolveJsonModule": true
  },
  "include": ["*.ts", "*.json"],
}

To generate the bundle

npx esbuild --bundle b.ts --format=esm > b.js

dts-bundle-generator command

I want to generate b.d.ts with dts-bundle-generator. I use

npx dts-bundle-generator b.ts

Expected output

Something like:

// Generated by dts-bundle-generator v8.0.1

export declare const x: {
	a: number;
	b: number;
	c: number[];
};

Actual output

b.d.ts(1,25): error TS2307: Cannot find module './a.json' or its corresponding type declarations.

Some solutions

Make a .json.d.ts file

If I make my own file called a.json.d.ts it works, but it doesn't get the types from the original json file. I looked into using MakeTypes or quicktype to generate this file but I was unable to get them to generate the correct thing directly. Also they are both pretty large dependencies.

Just let .json files through

The following patch works pretty well:

--- a/src/compile-dts.ts
+++ b/src/compile-dts.ts
@@ -84,6 +84,9 @@ function changeExtensionToDts(fileName: string): string {
        if (fileName.slice(-5) === '.d.ts') {
                return fileName;
        }
+       if (fileName.endsWith(".json")) {
+               return fileName;
+       }
 
        // .ts, .tsx
        const ext = path.extname(fileName);

I get the warning:

WARNING: It seems that some files in the compilation still are not declaration files.

but it generates the bundle I wanted. There seems to be additional trouble though because if I do:

export { a } from "./a.json"

the .d.ts file won't mention a.

Generate a .d.ts file from the json

It's also possible to generate a .d.ts file on the fly from the json as follows:

	const expression = file.statements[0].expression;
	const decls = expression.properties.map(prop => {
		const name = prop.name;
		if (!name) {
			return '';
		}
		if (!ts.isStringLiteral(name)) {
			throw new Error('Expected a string literal!');
		}
		const symbol = checker.getSymbolAtLocation(name);
		if (!symbol || !symbol.valueDeclaration) {
			return '';
		}
		const type = checker.getTypeOfSymbolAtLocation(symbol, symbol.valueDeclaration);
		return `export declare const ${name.text}: ${checker.typeToString(type)};`;
	});
    declarations.set(file.fileName.replace('.json', '.d.ts'), decls.join("\n"));

I think this is the best solution. I can make a PR if you like.

@timocov
Copy link
Owner

timocov commented Aug 27, 2023

It seems that the compiler team decided not to generate types for JSON files in microsoft/TypeScript#36066 (comment), so I don't think this tool should do it either as we're trying to leverage the provided API rather than building lots of add-ons on top of that.

As for your suggestion with adding json files to the compilation - while it looks good to me, it's probably worth adding a flag that would be disabled by default for this feature. The reason for this is that it would require your consumers to enable resolving json in their tsconfigs which they might not want or cannot do and having this feature enabled by default might have this unintentional side-effect.

@hoodmane
Copy link
Author

it would require your consumers to enable resolving json in their tsconfigs

In my case it does not require this since I am using esbuild to produce a bundle (.js and .d.ts) and downstream users use these bundles not the typescript itself. But I agree that it should be opt in. If you are not bundling the JSON then it produces an extra constraint on the downstream and this is an opportunity to remind people about that.

I think it might also be helpful to document the possibility of making a .json.d.ts file since for my use case this was good enough but I didn't notice that it was possible until I read the source code. So we could start with a more helpful error message mentioning this option.

@timocov
Copy link
Owner

timocov commented Aug 27, 2023

In my case it does not require this since I am using esbuild to produce a bundle (.js and .d.ts) and downstream users use these bundles not the typescript itself. But I agree that it should be opt in.

I meant, if a dts file would have references of json files then 1) you have to ship these json files along with your bundles in order of compilation work (but not for anything else) 2) enable resolveJsonModule option as otherwise the compiler in the consumer's project won't know how to find referenced files otherwise.

So we could start with a more helpful error message mentioning this option.

Sure. Do you have any ideas when it should be printed and what it should consist of? I'm asking because for me it would be kinda obvious but I'm too biased.

@hoodmane
Copy link
Author

hoodmane commented Aug 27, 2023

Do you have any ideas when it should be printed and what it should consist of?

I can try making a PR and we can see if you agree with what I write.

if a dts file would have references of json files

well yes but the resulting dts file doesn't reference the json files. If I start with a.json as follows:

{
    "a" : 2,
    "b": 7,
    "c": [1, 2, 3]
}

and b.ts:

import {a, b, c} from "./a.json";

const x = {a, b, c};
export {a, b, c, x};

then I get output files b.js:

// a.json
var a = 2;
var b = 7;
var c = [1, 2, 3];

// b.ts
var x = { a, b, c };
export { a, b, c, x };

and b.d.ts:

// Generated by dts-bundle-generator v8.0.1

export declare const a: number;
export declare const b: number;
export declare const c: number[];
export declare const x: {
	a: number;
	b: number;
	c: number[];
};

export {};

Neither makes any reference to a.json except in comments.

@hoodmane
Copy link
Author

I can try making a PR and we can see if you agree with what I write.

Though now I'm looking at the code and it's not obvious where to put it.

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

No branches or pull requests

2 participants