micheledaros / gnome-shell-extension-simulate-switching-workspaces-on-active-monitor

MIT License
24 stars 4 forks source link

Update README to reflect changes regarding TypeScript #10

Closed ProbstDJakob closed 3 months ago

ProbstDJakob commented 3 months ago

Hi @ProbstDJakob thanks for porting the extension to Typesctipt. Would you be so kind to update the README with detailed instructions on how to build it? I've tried on my system (with npm 10.8.1) and got many errors

_Originally posted by @micheledaros in https://github.com/micheledaros/gnome-shell-extension-simulate-switching-workspaces-on-active-monitor/pull/8#discussion_r1632075453_

ProbstDJakob commented 3 months ago

Sure, I will look into it. Since it worked on my computer (I don’t have it around so I do not know which version I have installed), could you please provide the error message and the steps, so I can hopefully reproduce it?

micheledaros commented 3 months ago

I tried to execute the target make submit. At first, npm complained that typescipt was not installed, and I fixed it with npm install -g typescript

Then I tried again and got this error


extension.ts:1:21 - error TS2307: Cannot find module 'gi://Clutter' or its corresponding type declarations.

1 import Clutter from "gi://Clutter";
                      ~~~~~~~~~~~~~~

extension.ts:2:19 - error TS2307: Cannot find module 'gi://Shell' or its corresponding type declarations.

2 import Shell from "gi://Shell";
                    ~~~~~~~~~~~~

extension.ts:3:16 - error TS2307: Cannot find module 'gi://St' or its corresponding type declarations.

3 import St from "gi://St";
                 ~~~~~~~~~

extension.ts:4:18 - error TS2307: Cannot find module 'gi://Meta' or its corresponding type declarations.

4 import Meta from "gi://Meta";
                   ~~~~~~~~~~~

extension.ts:5:17 - error TS2307: Cannot find module 'gi://Gio' or its corresponding type declarations.

5 import Gio from "gi://Gio";
                  ~~~~~~~~~~

extension.ts:7:23 - error TS2307: Cannot find module 'resource:///org/gnome/shell/ui/main.js' or its corresponding type declarations.

7 import * as Main from "resource:///org/gnome/shell/ui/main.js";
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

extension.ts:8:28 - error TS2307: Cannot find module 'resource:///org/gnome/shell/ui/panelMenu.js' or its corresponding type declarations.

8 import * as PanelMenu from "resource:///org/gnome/shell/ui/panelMenu.js";
                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

extension.ts:9:28 - error TS2307: Cannot find module 'resource:///org/gnome/shell/ui/popupMenu.js' or its corresponding type declarations.

9 import * as PopupMenu from "resource:///org/gnome/shell/ui/popupMenu.js";
                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

extension.ts:10:25 - error TS2307: Cannot find module 'resource:///org/gnome/shell/extensions/extension.js' or its corresponding type declarations.

10 import {Extension} from "resource:///org/gnome/shell/extensions/extension.js";
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

extension.ts:11:28 - error TS2307: Cannot find module 'gettext' or its corresponding type declarations.

11 import {gettext as _} from "gettext";
                              ~~~~~~~~~

extension.ts:145:16 - error TS2304: Cannot find name 'global'.

145         return global.get_window_actors().map((x) => new WindowWrapper(x));
                   ~~~~~~

extension.ts:145:48 - error TS7006: Parameter 'x' implicitly has an 'any' type.

145         return global.get_window_actors().map((x) => new WindowWrapper(x));
                                                   ~

extension.ts:156:30 - error TS2304: Cannot find name 'global'.

156         let currentMonitor = global.display.get_current_monitor();
                                 ~~~~~~

extension.ts:162:28 - error TS2304: Cannot find name 'global'.

162         this.nWorkspaces = global.workspace_manager.get_n_workspaces();
                               ~~~~~~

extension.ts:173:16 - error TS2304: Cannot find name 'global'.

173         return global.workspace_manager.get_active_workspace_index();
                   ~~~~~~

extension.ts:315:9 - error TS2584: Cannot find name 'console'. Do you need to change your target library? Try changing the 'lib' compiler option to include 'dom'.

315         console.log(value);
            ~~~~~~~

extension.ts:337:64 - error TS2339: Property 'getSettings' does not exist on type 'SwitchWorkspacesExtension'.

337         let controller = new Controller(workspaceService, this.getSettings());
                                                                   ~~~~~~~~~~~

extension.ts:339:40 - error TS2304: Cannot find name 'global'.

339         let workSpaceChangedListener = global.workspace_manager.connect("active-workspace-changed", () => {
                                           ~~~~~~

extension.ts:365:9 - error TS2304: Cannot find name 'global'.

365         global.workspace_manager.disconnect(this.state.workSpaceChangedListener);
            ~~~~~~

Found 19 errors in the same file, starting at: extension.ts:1
ProbstDJakob commented 3 months ago

Ok I see what the problem is. The npm i -g typescript is unnecessary. It just installs TypeScript globally, but this must not match the requirements of a project (in this case it probably works, I added the latest as dependency). To install the dependencies of a npm project you have to navigate to the directory containing the package.json (in this case the simulate-switching-workspaces-on-active-monitor@micheledaros.com) and run npm i (short for npm install). This will install all the dependencies (including the dev dependencies) specified in the package.json with the required versions (without installing it globally - no -g flag). \ If you are planning to use a CI/CD the better suiting command would be npm ci (short for npm clean-install). This will install the versions specified in the package-lock.json.

I will add a short description on how to bootstrap the project and create a PR later.

micheledaros commented 3 months ago

Thanks. I've fixed the Makefile, so it executes automatically npm ci during the build phase. Here's the PR https://github.com/micheledaros/gnome-shell-extension-simulate-switching-workspaces-on-active-monitor/pull/11

If you manage please have a look, I'm neither an expert in Type/Javascript nor in Makefile :grimacing:

It would make sense to add to the README a sentence about the required npm version

ProbstDJakob commented 3 months ago

If you are not familiar with Makefiles and there is no other reason for it, I would suggest dropping it and replace it with npm-scripts. This way you only need one tool. If it is ok for you, I would open an PR which moves the scripts from the Makefile into the package.json . Will there be a need to make the scripts also work on Windows or is this a problem for a future contributor?

I will create a PR (or include it in the above mentioned PR) which will include this information, but as long as the version is not five years old, there should be no problem.

micheledaros commented 3 months ago

Getting rid of makefile sounds good to me, feel free to open a PR.

I don't see the need for supporting windows, since gnome does not support windows.

ProbstDJakob commented 3 months ago

Well good point. Maybe I should get some rest ^^

I will open a PR in the next days.

ProbstDJakob commented 3 months ago

... or I do it right away ^^

I have tested all npm run * commands except for npm run install because I did not want to mess around with my already running installation. It should work, but it would be better if you could test it.