Type conflicts with lib.dom.d.ts

[HeavenVolkoff]

Hello,

Currently, the type definition for the global console is commented out in:
https://github.com/sammydre/ts-for-gir/blob/979d2a016f182ca54f14047483d2b29cac126ade/templates/Gjs/index.d.ts#L201-L202

This is due to conflicts with the global definitions shipped with typescript, that are used by most IDEs (such as VSCode):
https://github.com/microsoft/TypeScript/blob/93f2d2b9a1b2f8861b49d76bb5e58f6e9f2b56ee/lib/lib.dom.d.ts#L17750

However, this results in gjs code being linted with lib.dom.d.ts type definitions, which is not ideal, as gjs implementation of those interfaces has slight differences and incompatibilities with the DOM implementation present in browsers, and the vast majority of the DOM API is not implemented in gjs (and probably will not be, according to #265).

This can be worked around by using a custom tsconfig.json / jsconfig.json that doesn't contain DOM in the lib property:

{
  "compilerOptions": {
    "lib": ["esnext"],
    "allowJs": true,
    "checkJs": true
  },
  "include": ["src/**/*"],
  "exclude": ["node_modules"],
  "$schema": "https://json.schemastore.org/tsconfig"
}

So far in my tests with VSCode, this prevents the lib.dom.d.ts from being used, and allows the local definitions generated by ts-for-gir to work without error, even with console commented out and my, yet not upstreamed, additions of typing definition for gjs recently implemented Text(D)Encoder .

IMHO, ts-for-gir could attempt to solve this automatically. The logic I am considering would be something along the lines of:

• If the project where the definitions are being generated doesn't have a tsconfig.json than generate one following the example above
• If the project already has a tsconfig.json, check if DOM is included in the lib property:
  • If it is not, then do nothing and continue
  • If it is, either:
    • Throw an error and stop
    • Throw a warning and keep current behaviour (with conflicting globals not being included in the generated types)

[JumpLink]

Hello @HeavenVolkoff,

if you have good results with it then you can already make a pull request from your branch, best is to add a note about the tsconfig configuration to README.md. Then we can already merge that. Your mentioned problem with the tsconfig we can then also still solve afterwards.

I like your suggestion, but what do you think about adding an new init CLI command with which a new project can be created instead.

The init command then ask:

• Gjs or node-gtk
• CJS or ESM

To keep the command from getting too complex we should concentrate on the most important for now, if there is a need for more configuration options we can always introduce them, so e.g. I would make esbuild to the default bundler and not ask for another.

The init command then does the following:

• Runs npm init
• Parses the created package.json (in this way we also have the information that was entered)
• Rebuilds the package.json by
  • Adding some default scripts to the package.json (like build:types, build:app, ..)
  • Adding esbuild to the devDependencies
  • Adding ts-for-gir to the devDependencies (as soon as I have create a new npm package (I think I will name it @ts-for-gir/cli))
• Creates a tsconfig.json (with your suggestion to exclude the DOM types)
• Creates a esbuild.js
• Creates a .ts-for-girrc.js with the chosen
  • environments: "gjs" | "node"
  • moduleType: "esm" | "commonjs"
• Creates a src/index.ts as the starting point with a simple example

What do you think?

[HeavenVolkoff]

if you have good results with it then you can already make a pull request from your branch, best is to add a note about the tsconfig configuration to README.md. Then we can already merge that. Your mentioned problem with the tsconfig we can then also still solve afterwards.

You mean the Text(D)Encoder implementation? If so, I can open a PR with the global definition commented out and a README note mentioning the problem about tsconfig for now.

I like your suggestion, but what do you think about adding an new init CLI command with which a new project can be created instead.

I agree, that is a better approach.

To keep the command from getting too complex we should concentrate on the most important for now, if there is a need for more configuration options we can always introduce them, so e.g. I would make esbuild to the default bundler and not ask for another.

Agree, I also use esbuild on my gjs projects.

The init command then does the following:

• Runs npm init

• Parses the created package.json (in this way we also have the information that was entered)

• Rebuilds the package.json by

• Adding some default scripts to the package.json (like build:types, build:app, ..)

• Adding esbuild to the devDependencies

• Adding ts-for-gir to the devDependencies (as soon as I have create a new npm package (I think I will name it @ts-for-gir/cli))

Currently, I am using ts-for-gir as a git submodule in my projects, we can use this solution for now, or just wait for the package to be published before implementing the init command. IMHO, @ts-for-gir/cli is a good name for the package.

• Creates a tsconfig.json (with your suggestion to exclude the DOM types)

• Creates a esbuild.js

• Creates a .ts-for-girrc.js with the chosen
  * environments: "gjs" | "node"
  * moduleType: "esm" | "commonjs"

• Creates a src/index.ts as the starting point with a simple example

The rest of logic seems fine with me. However, I still think the generate command should at least warn users that already have an initialized project that includes the DOM lib in tsconfig.json.

As soon as I have some more free time, I will try to start implementing this, if that is ok with you?

[JumpLink]

As soon as I have some more free time, I will try to start implementing this, if that is ok with you?

@HeavenVolkoff Jea, feel free to implement that :)

[JumpLink]

You mean the Text(D)Encoder implementation? If so, I can open a PR with the global definition commented out and a README note mentioning the problem about tsconfig for now.

@HeavenVolkoff Perhaps I have now also misunderstood something, if the tsconfig is set correctly, then the types for console and text(D)encoder do not cause any problems anymore, do they? If not, I think you don't need to comment out the types (except for node-gtk) and the note in the README should be enough. Or did you think It's better that you implement the tsconfig check before?

[swsnr]

@JumpLink @HeavenVolkoff #86 introduces an interactive prompt which asks whether DOM types should be ignored if the generator believes that the DOM library is enabled in tsconfig. I can't find a way to disable this prompt, or non-interactively answer it with "no".

But I'd like to, because the way the typescript configuration is checked doesn't really work for multi-project setups, where the top-level tsconfig.json typically only has references to subprojects, and the subprojects then have their own tsconfig which inherit compiler options from a shared base config. In other words, the actual compiler flags aren't reachable from my out directory.

I don't think it's a good idea to prompt interactively in this situation (no code generator that's supposed to be used in some kind of build system should ever prompt, in my opinion); I'd prefer if I only got a warning, which advised to change the configuration, and had a way to suppress that warning.

Would you accept a pull request to this end?

[HeavenVolkoff]

@swsnr it is possible to preemptively answer this, and any prompts, with command line arguments or entries in the config file. In the case of the DOM lib prompt there is --noDOMLib. Maybe this is not the best argument name, but it should be explained in the ts-for-gir --help.

About the multi-project setup, the currently implemented logic should work in those cases. Would it be possible for you to share a simple example of this not working, maybe there is a bug in the get-tsconfig lib that is used internally by ts-for-gir to parse .tsconfig files.

@JumpLink maybe a check should be implemented for when ts-for-gir is executed in a non interactive environment so that it either assume some default behaviour or raise an error instead of prompting.

[swsnr]

@HeavenVolkoff --noDOMLib covers the "yes" part; how would I answer "no" in advance? I tried to set noDOMLib: false explicitly, even though that–being the default–should be redundant, but still got the prompt. Which in my reading of the source code is to be expected, so I don't think it's possible to force a "no" answer in advance, if tsconfig doesn't have "compilerOptions".

And I think that's a pretty common situation in a multi-project build of the following schema: One top-level tsconfig.json which has only references to subprojects, one top-level tsconfig.base.json (or similar) with shared compiler options, and then a tsconfig.json per subproject in a subdirectory with extends the base config.

I think that's a pretty standard setup, to be found in e.g. the typescript compiler repository itself.

In this layout ts-for-gir will only see the top-level tsconfig.json when asked to generate types to a top-level directory, i.e. it'll just be design never see the compiler options.

I don't think that this is an exotic setup, but it's not supported by ts-for-gir now, because I cannot force noDOMLib to false. Ideally, I'd like for ts-to-gir to stop checking my tsconfig.json; I don't think it's any of its business. It should just generate accurate types.

[HeavenVolkoff]

@swsnr you are right, --noDOMLib does not cover the use case where it is required to set in advance that ts-for-gir should not generate any conflicting types. This is an oversight and I will implement a fix for it.

About ts-for-gir interacting with tsconfig, IMHO I think it is necessary, considering one of its major use cases is for the generated types to be used for type checking, and most (if not all) currently used linters interact with tsconfig for figuring out the runtime environment and available global symbols...

Regarding the tsconfig discovery logic, currently it is impelemted by searching for the first tsconfig file in the given output directory or any of its parents, and the lib that is used to parse tsconfig should support extends. Is your output directory for ts-for-gir generate outside of where the relevant tsconfig file is located? If that is the case maybe a new argument that allows for the tsconfig path to be explicit given should solve this, I will try and implement it too.

[swsnr]

About ts-for-gir interacting with tsconfig, IMHO I think it is necessary,

I think it's fundamentally wrong, because it inverts facts and potentially hides type errors.

The gjs environment is what it is regardless of what I have in my tsconfig.json or in my ts-for-gir configuration. If I have the DOM types in the former, my tsconfig.json is wrong, not the generated types. It's not as if there was a choice for ts-for-gir here: Answering "yes" in the prompt generates incomplete types for gjs, and the typescript compiler will silently and with no further warning or error use the wrong types for some names.

I think ts-for-gir must always generate complete types, and tsc must fail if I have conflicting types.

[swsnr]

Or am I misunderstanding the meaning of "noDOMLib" and its interaction with the DOM lib types specified in tsconfig?

[JumpLink]

@JumpLink maybe a check should be implemented for when ts-for-gir is executed in a non interactive environment so that it either assume some default behaviour or raise an error instead of prompting.

@HeavenVolkoff So far, user interactions have been prevented by setting --ignoreVersionConflicts, I agree to introduce a general option for this.

I think ts-for-gir must always generate complete types, and tsc must fail if I have conflicting types.

@swsnr Your argument makes sense, and therefore it should be the default behavior. However, currently I am working on a wrapper to replicate the DOM API in Gjs, and it is possible that I still need the option to be able to use the DOM types for it.