Executing a deeper penetration into Google's open-source project
Tags: webdev, programming, opensource, typescript
Published: 2023-11-28 14:02:37

## Issues
1. [Enable strict mode for workspace-search](https://github.com/google/blockly-samples/issues/2032)
2. [Enable strict mode for workspace-backpack](https://github.com/google/blockly-samples/issues/2030)

## Pull Requests
1. [feature: Enable strict mode for workspace search #2032](https://github.com/google/blockly-samples/pull/2079)
2. [feature: enabled TS strict mode in workspace-backapck, resolved TS errors](https://github.com/google/blockly-samples/pull/2080)

## Issue scope
Both the above issues I worked on dealt with 2 different parts of the project.

* workspace-search plugin - allows a user to search for items present within the blocks on the current workspace. [An interactive sample can be found here](https://google.github.io/blockly-samples/plugins/workspace-backpack/test/index.html). Looks like this:

>![workspace-search](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/gwonmh4fpcde87b7xqnf.gif)

* workspace-backpack plugin - allows a user to save current blocks on the workspace to a backpack. [An interactive sample can be found here](https://google.github.io/blockly-samples/plugins/workspace-search/test/index.html). Looks like this:

>![workspace-backpack](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/27c0zwcrkgr4kgsgwwoz.gif)

## Instructions
The instructions on how to work and the testing to be performed for these issues were the same for both of them. 

1. Run `npm install` at the root of Blockly samples to set up dependencies.
2. Navigate to the plugin's directory
3. Change `strict: false` to `strict: true` in the plugin's tsconfig.json
4. Run `npm run build` and see if there are any TypeScript errors.
5. Resolve any TypeScript errors, if present. If not, then we got lucky and accidentally wrote strict code! Yay!
6. Run `npm run build` again to make sure there are no TypeScript errors.
7. Run `npm run start` and play with the plugin in the playground to make sure the plugin still behaves as expected.
8. Run `npm run test` to run any automated tests.
9. Run `npm run lint` to make sure there are no formatting errors.
10. Open a PR with your changes!

## TS error list
1. Received 16 TS errors for [Enable strict mode for workspace-search](https://github.com/google/blockly-samples/issues/2032) issue, after turning the strict flag from false to true. The errors are listed below:

```javascript
[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(136,50)
      TS2345: Argument of type '(evt: KeyboardEvent) => void' is not assignable to parameter of type '(event: Event) => void'.
  Types of parameters 'evt' and 'event' are incompatible.
    Type 'Event' is missing the following properties from type 'KeyboardEvent': altKey, charCode, code, ctrlKey, and 17 more.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(153,55)
      TS2345: Argument of type '(evt: KeyboardEvent) => void' is not assignable to parameter of type '(event: Event) => void'.
  Types of parameters 'evt' and 'event' are incompatible.
    Type 'Event' is not assignable to type 'KeyboardEvent'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(159,7)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(223,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(294,41)
      TS2345: Argument of type '(e: KeyboardEvent) => void' is not assignable to parameter of type '(event: Event) => void'.
  Types of parameters 'e' and 'event' are incompatible.
    Type 'Event' is not assignable to type 'KeyboardEvent'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(331,7)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(334,9)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(336,9)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(339,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(347,26)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(366,28)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(440,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(461,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(524,13)
      TS7034: Variable 'topBlockText' implicitly has type 'any[]' in some locations where its type cannot be determined.

[tsl] ERROR in blockly-samples/plugins/workspace-search/src/workspace_search.ts(530,19)
      TS7005: Variable 'topBlockText' implicitly has an 'any[]' type.

[tsl] ERROR in blockly-samples/plugins/workspace-search/test/index.ts(42,22)
      TS2345: Argument of type 'HTMLElement | null' is not assignable to parameter of type 'HTMLElement'.
  Type 'null' is not assignable to type 'HTMLElement'.
```

2. Received 45 TS errors for [Enable strict mode for workspace-backpack](https://github.com/google/blockly-samples/issues/2030) issue, after turning the strict flag from false to true. The errors are listed below:

```javascript
[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(139,26)
      TS2345: Argument of type 'BackpackContextMenuOptions | undefined' is not assignable to parameter of type 'BackpackContextMenuOptions'.
  Type 'undefined' is not assignable to type 'BackpackContextMenuOptions'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(170,7)
      TS2322: Type '{ [rendererConstant: string]: any; } | null' is not assignable to type '{ [rendererConstant: string]: any; } | undefined'.
  Type 'null' is not assignable to type '{ [rendererConstant: string]: any; } | undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(186,26)
      TS18047: 'HorizontalFlyout' is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(197,26)
      TS18047: 'VerticalFlyout' is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(201,5)
      TS18047: 'parentNode' is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(257,7)
      TS2345: Argument of type 'SVGElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(262,19)
      TS2345: Argument of type 'SVGElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(263,19)
      TS2345: Argument of type 'SVGElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(264,19)
      TS2345: Argument of type 'SVGElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(410,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(465,36)
      TS7006: Parameter 'json' implicitly has an 'any' type.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(465,42)
      TS7006: Parameter 'keys' implicitly has an 'any' type.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(624,7)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(630,7)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(668,12)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(679,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(701,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(711,5)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(803,34)
      TS2345: Argument of type 'SVGImageElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack.ts(805,37)
      TS2345: Argument of type 'SVGImageElement | null' is not assignable to parameter of type 'Element'.
  Type 'null' is not assignable to type 'Element'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(26,3)
      TS2322: Type '(menuOptions: ContextMenuOption[], e: PointerEvent) => void' is not assignable to type '(menuOptions: ContextMenuOption[], e: Event) => void'.
  Types of parameters 'e' and 'e' are incompatible.
    Type 'Event' is missing the following properties from type 'PointerEvent': height, isPrimary, pointerId, pointerType, and 33 more.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(30,23)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(61,18)
      TS18048: 'scope.block' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(66,25)
      TS2531: Object is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(73,24)
      TS18048: 'scope.block' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(73,24)
      TS18047: 'scope.block.workspace.targetWorkspace' is possibly 'null'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(76,28)
      TS2345: Argument of type 'BlockSvg | undefined' is not assignable to parameter of type 'Block'.
  Type 'undefined' is not assignable to type 'Block'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(108,18)
      TS18048: 'scope.block' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(117,41)
      TS2345: Argument of type 'BlockSvg | undefined' is not assignable to parameter of type 'Block'.
  Type 'undefined' is not assignable to type 'Block'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(123,24)
      TS18048: 'scope.block' is possibly 'undefined'.

[tsl] ERROR in DPI/blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(126,25)
      TS2345: Argument of type 'BlockSvg | undefined' is not assignable to parameter of type 'Block'.
  Type 'undefined' is not assignable to type 'Block'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(133,49)
      TS2345: Argument of type '{ displayText: (scope: Scope) => string | undefined; preconditionFn: (scope: Scope) => "enabled" | "hidden" | "disabled"; callback: (scope: Scope) => void; scopeType: ScopeType; id: string; weight: number; }' is not assignable to parameter of type 'RegistryItem'.
  Types of property 'displayText' are incompatible.
    Type '(scope: Scope) => string | undefined' is not assignable to type 'string | ((p1: Scope) => string)'.
      Type '(scope: Scope) => string | undefined' is not assignable to type '(p1: Scope) => string'.
        Type 'string | undefined' is not assignable to type 'string'.
          Type 'undefined' is not assignable to type 'string'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(148,12)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(149,26)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(158,24)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(161,26)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(192,12)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(193,26)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(202,24)
      TS18048: 'ws' is possibly 'undefined'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(209,11)
      TS2345: Argument of type 'WorkspaceSvg | undefined' is not assignable to parameter of type 'Workspace'.
  Type 'undefined' is not assignable to type 'Workspace'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(219,49)
      TS2345: Argument of type '{ displayText: (scope: Scope) => string | undefined; preconditionFn: (scope: Scope) => "enabled" | "hidden"; callback: (scope: Scope) => void; scopeType: ScopeType; id: string; weight: number; }' is not assignable to parameter of type 'RegistryItem'.
  Types of property 'displayText' are incompatible.
    Type '(scope: Scope) => string | undefined' is not assignable to type 'string | ((p1: Scope) => string)'.
      Type '(scope: Scope) => string | undefined' is not assignable to type '(p1: Scope) => string'.
        Type 'string | undefined' is not assignable to type 'string'.
          Type 'undefined' is not assignable to type 'string'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/backpack_helpers.ts(239,28)
      TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
  Type 'undefined' is not assignable to type 'boolean'.

[tsl] ERROR in blockly-samples/plugins/workspace-backpack/src/ui_events.ts(50,5)
      TS7053: Element implicitly has an 'any' type because expression of type '"isOpen"' can't be used to index type 'AbstractEventJson'.
  Property 'isOpen' does not exist on type 'AbstractEventJson'.
```

## Execution
The way to go about these issues were to,
1. Fix all the errors without thinking about the code quality.
2. Once all errors are fixed, improve code quality while still testing to make sure the code changes haven't introduced new TS errors or ESLint warnings and haven't affected the behaviour of the plugin.

Working on code quality was my utmost priority for these PRs because they take code quality seriously and I know this because I’ve contributed to this repo before for 0.3 release and also went through a few PRs submitted by others to read the maintainers' reviews.

It took me about 3 days of back-and-forth reading, understanding and implementing code changes before I was able to create a PR for both these issues which included multiple commits on these PRs and a pipeline failure for one of the PR for which I provided a separate commit [detailed here](https://github.com/google/blockly-samples/pull/2080#issuecomment-1818023720). I regularly visited TS documentation and Stack Overflow websites to get a better understanding of the errors.

I was confident about most of the code changes in both these PRs except for a couple of them. I decided to implement the changes as per my understanding and if there's a better way to write that specific implementation, I'd bring them up in the conversation on the PR or in the description of the PR to bring it to their attention.

* For PR [feature: Enable strict mode for workspace search #2032](https://github.com/google/blockly-samples/pull/2079) couple of code changes I was not confident about were:

>On [L153](https://github.com/google/blockly-samples/pull/2079/files#diff-5c7d57d9f93392819388fd3b78392ecc826b60fcceb18890af2d8178a262c2e7L153), I received error,

>```typescript
TS2345: Argument of type '(e: KeyboardEvent) => void' is not
assignable to parameter of type '(event: Event) => void'.
  Types of parameters 'e' and 'event' are incompatible.
    Type 'Event' is not assignable to type 'KeyboardEvent'.
```

>which relates to declaration of a function called `addEvent` on [L198-203](https://github.com/google/blockly-samples/pull/2079/files#diff-5c7d57d9f93392819388fd3b78392ecc826b60fcceb18890af2d8178a262c2e7L198-L203). As the error mentions, the `addEvent` function declaration takes a function as an argument, whose argument is of type `Event`, however when `addEvent` is called, it takes in the event type as `KeyboardEvent`. TS isn't able to automatically infer at build time that `KeyboardEvent` is sub-type of type `Event` after all so I [implemented](https://github.com/google/blockly-samples/pull/2079/files#diff-5c7d57d9f93392819388fd3b78392ecc826b60fcceb18890af2d8178a262c2e7R198) the function with the use of generics by extending the type `T` from `Event` so at run time it can automatically infer that the `KeyboardEvent` type used is indeed the correct one. I was initially planning to perform a check of `if (evt instanceof KeyboardEvent)` only then execute the rest of the code but the implementation makes the code inflexible so I went the route of generics. If in future another argument of type `Function` is added to the `addEvent` function which takes in a different type of event this code can handle it gracefully since it already extend type `Event`. If the maintainer proposes a different implementation I'll just go for that.

>On [L40-43](https://github.com/google/blockly-samples/pull/2079/files#diff-b0dd3df082f16f5f78be79382a91c7d5fab1c2967ea213d2fcc1f38897925330R40-R43) I received error,

>```typescript
TS2345: Argument of type 'HTMLElement | null' is not
assignable to parameter of type 'HTMLElement'.
  Type 'null' is not assignable to type 'HTMLElement'.
```

> This error has to do with type assignment mismatch where the code `document.getElementById('root')` would return either a value of type `HTMLElement` or `null`. However the `createPlayground` function expects an argument of type `HTMLElement`. So I [implemented](https://github.com/google/blockly-samples/pull/2079/files#diff-b0dd3df082f16f5f78be79382a91c7d5fab1c2967ea213d2fcc1f38897925330R40-R43) the use of type guards by checking whether the returned value from `createPlayground` function is an instance of `HTMLElement` interface at runtime. If it is then execute the rest of the code within if block. The issue description mentioned that they generally avoid type casting like `variableName as type` so I made use of `instanceOf` type guard since I also saw it being used elsewhere in the codebase. If the maintainer proposes a different implementation I'll just go for that.

For PR [feature: enabled TS strict mode in workspace-backapck, resolved TS errors](https://github.com/google/blockly-samples/pull/2080) a code change I was not confident about was:

> On [L465](https://github.com/google/blockly-samples/pull/2080/files#diff-363096547bdd4e0d53f88eab1dd90badc5858e2d18f792b008abb42d94e375b1L465) I received 2 errors,

>```typescript
TS7006: Parameter 'json' implicitly has an 'any' type.
```
>```typescript
TS7006: Parameter 'keys' implicitly has an 'any' type.
```

> Adding `any` as both of their types wasn't allowed. So I assigned it a type of `Blockly.serialization.blocks.State` for the `json` argument variable and `string[]` for the `keys` argument variable. Type assignment for `keys` worked, however for `json`, when being used as `json[key]`, another warning popped up,

>```typescript
Element implicitly has an 'any' type because expression
 of type 'string' can't be used to index type 'State'.
  No index signature with a parameter of type 'string'
 was found on type 'State'.ts(7053)
```

>State interface definition
>```typescript
export interface State {
    type: string;
    id?: string;
    x?: number;
    y?: number;
    collapsed?: boolean;
    deletable?: boolean;
    movable?: boolean;
    editable?: boolean;
    enabled?: boolean;
    inline?: boolean;
    data?: string;
    extraState?: any;
    icons?: {
        [key: string]: any;
    };
    fields?: {
        [key: string]: any;
    };
    inputs?: {
        [key: string]: ConnectionState;
    };
    next?: ConnectionState;
}
```

> As per my understanding, what this error means is that TS isn't able to figure out that the usage of `json` variable in this form, `json[key];` to extract out the value of the key is correct. Considering the `State` interface's properties are keys with different names, TS isn't able to infer that they can be used as strings to get the value of those keys. So I thought of creating a [local interface](https://github.com/google/blockly-samples/pull/2080/files#diff-363096547bdd4e0d53f88eab1dd90badc5858e2d18f792b008abb42d94e375b1R983-R985) part of this file which extends from `State` interface whose sole property would be `[key: string]: unknown`. This property declaration means for an object that represents this interface, there are multiple keys which would be of type string and their values would be of type unknown since we can't figure out what is the type of the value at compile-time and this solved my issue. It resolved all the TS errors plus the ESLint warnings. If the maintainer proposes a different implementation I'll just go for that.

The above examples for both these PRs were the most difficult and least confident out of all for me, but that doesn't mean other code changes were simple and straightforward. I'd have to take constant breaks between the coding session when I was writing code for this PR to process all that I've learnt from reading documentation and my understanding of the codebase to apply that in practise.

I'm hoping for a favourable outcome from these 2 PRs. For all the changes I made for these PRs, I successfully achieved both my goals which were about diving deep into the project's codebase and interfacing closely with TS code.