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

Add typechecking for TypeScript #946

Merged
merged 24 commits into from
Jul 6, 2024
Merged

Add typechecking for TypeScript #946

merged 24 commits into from
Jul 6, 2024

Conversation

vixalien
Copy link
Contributor

@vixalien vixalien commented Jun 18, 2024

This is a draft because there are 3 outstanding issues:

1. It seems the typechecking really works when you open a previously-saved project

I believe this is because when you open a new project and switch to typescript, the setupTypeScriptProject will copy the necessary files and folders (tsconfig.json, types/ambient.d.ts and gi-types/*) to the project. The language server doesn't know that the "project changed" (newly created files) so it won't see the various type augmentations.

The fix could be as easy as to kill and spawn a new language server, but I'm not sure if that would be the right approach.

2. The globalThis.workbench types aren't there yet

Working on it 🚀

3. Generated gi.d.ts files require setting the path to include them

The generated types from gi-typescript-definitions have peculiar imports. They import each other as if they are all global

import * as Gtk from "gtk4";
import * as GObject from "gobject2";
import * as Gio from "gio2";
import * as GLib from "glib2";
import * as Gdk from "gdk4";
import * as Pango from "pango1";
import * as Gsk from "gsk4";

This doesn't work because unless you add the following to tsconfig.json

    "paths": {
      "*": ["*", "gi-types/*"]
    },

However, this means that the typechecker will think all the types in gi-types are "global" types, hence you get completions from all types in the gi-types folder.

image

@vixalien vixalien requested a review from sonnyp as a code owner June 18, 2024 14:46
@vixalien vixalien marked this pull request as draft June 18, 2024 14:46
@vixalien
Copy link
Contributor Author

With the recent commits, the first issue #1 is fixed.

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So about the types - I think they should be part of Workbench, not of the project.

The reason for that is that they are "tied" to the environment Workbench provides.
Can you look into passing a custom path for the types to typescript-language-server and pointing to the directory in /usr/share ?

In a way that we don't need setupTypeScriptProject anymore.

For Rust and the cargo file it's a bit different - I don't 100% remember why we did it this way but I recall there were good reasons

Ideally in the future, types will be part of org.gnome.Sdk and we won't need a copy in Workbench anymore.

src/lsp/LSPClient.js Show resolved Hide resolved
@vixalien
Copy link
Contributor Author

The reason for that is that they are "tied" to the environment Workbench provides.
Can you look into passing a custom path for the types to typescript-language-server and pointing to the directory in /usr/share ?

I think the correct way is to change the tsconfig.json dynamically to point to the installed g-types.

@sonnyp
Copy link
Contributor

sonnyp commented Jul 2, 2024

Note from our discussion: tsconfig can remain part of the project for now if that's easier and we can revisit that later

@vixalien vixalien changed the title Draft: Add typechecking for TypeScript Add typechecking for TypeScript Jul 3, 2024
@vixalien vixalien marked this pull request as ready for review July 3, 2024 11:31
@vixalien vixalien requested a review from sonnyp July 3, 2024 11:31
@vixalien
Copy link
Contributor Author

vixalien commented Jul 3, 2024

CI seems to be failing because of the Freedesktop SDK 24.08beta situation

src/langs/typescript/TypeScriptDocument.js Show resolved Hide resolved
src/langs/typescript/TypeScriptDocument.js Show resolved Hide resolved
src/langs/typescript/types/ambient.d.ts Outdated Show resolved Hide resolved
src/langs/typescript/typescript.js Outdated Show resolved Hide resolved
src/langs/typescript/typescript.js Outdated Show resolved Hide resolved
src/langs/typescript/typescript.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
src/langs/typescript/template/meson.build Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
@vixalien vixalien requested a review from sonnyp July 3, 2024 15:49
@sonnyp sonnyp merged commit 2b13cf3 into main Jul 6, 2024
1 check passed
@sonnyp sonnyp deleted the wip/vixalien/typechecking branch July 6, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants