DEV Community: Soham Thaker

Evaluating a deeper penetration into Google's open-source project

Soham Thaker — Tue, 28 Nov 2023 14:02:56 +0000

Code Review

A lot of conversation regarding code review happened as you can notice from the PRs itself.

PR #1 - feature: Enable strict mode for workspace search #2032

For this PR most of the reviews were about replacing the conditional check with optional chaining for better code readability. A couple of the reviews were about praising my code changes and writing a better if conditional check.

Links to the reviews are here:

use optional chaining

praised usage of generics

use optional chaining

better if conditional check for readability

use optional chaining

They approved both my code changes for this PR which I was skeptical about as I mentioned in the previous blog post so that was a major win for me. Besides, I was curious about their review regarding why they prefer optional chaining in certain places and not for the others so I asked them to give me more information about the same. They provided me with an insightful reason why they chose optional chaining for certain scenarios. The conversation can be found here.

PR #2 - feature: enabled TS strict mode in workspace-backapck, resolved TS errors

For this PR, I received all sorts of reviews considering this PR had quite a lot of code changes compared to PR #1.

Links to the reviews are here:

throwing an error when interface not found

using optional chaining

confirming the changes are good

trying a different approach for declaring types of args

using optional chaining

praising the code changes

early return when the value is not found

Most of the changes asked during the PR review were straightforward. They provided a review for the code change I was skeptical about as mentioned in the previous blog for this PR. They mentioned using Blockly.serialization.blocks.State type by itself for json argument but I'd already tried that before putting out the PR which I explained to them in this conversation. They eventually agreed to my reasoning and approved my code change after I presented them with my findings, so that was a major win for me.

Overall this release was a 10/10 success for me in terms of achieving my goals and the outcome that I was looking for. They provided prompt reviews for my PR and merged it quickly as well and the changes were released for their respective releases, v8.1.0 for workspace-search and v5.3.0 for workspace-backpack

I am quite happy with my progress in this course. From being skeptical and doubting whether I could take on this massive challenge of contributing to an open-source project to actually contributing towards multiple of these open-source projects, was one hell of a journey. Thank you Professor David for teaching us Seneca College students one of the most vital skills in becoming a better developer. I will forever be grateful for these invaluable skills that you provided me with, helped me navigate this course and aided me in building confidence in myself that I can surely get any task done if I'm committed to pursuing it.

Executing a deeper penetration into Google's open-source project

Soham Thaker — Tue, 28 Nov 2023 14:02:37 +0000

Issues

Pull Requests

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. Looks like this:

workspace-backpack plugin - allows a user to save current blocks on the workspace to a backpack. An interactive sample can be found here. Looks like this:

Instructions

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

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

TS error list

Received 16 TS errors for Enable strict mode for workspace-search issue, after turning the strict flag from false to true. The errors are listed below:

[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'.

Received 45 TS errors for Enable strict mode for workspace-backpack issue, after turning the strict flag from false to true. The errors are listed below:

[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,

Fix all the errors without thinking about the code quality.
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. 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 couple of code changes I was not confident about were:

On L153, I received error,
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. 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 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 I received error,
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 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 a code change I was not confident about was:

On L465 I received 2 errors,
TS7006: Parameter 'json' implicitly has an 'any' type.
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,
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
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 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.

Planning a deeper penetration into Google's open-source project

Soham Thaker — Tue, 21 Nov 2023 05:00:00 +0000

Project Details

I chose the same project I chose during the 0.3 release, named blockly-samples for the 0.4 release, essentially to build upon my knowledge. Initially, I searched for other projects to contribute to, to expand my knowledge but a couple of things pulled me right back to this project. One is the way they do code reviews. Their PR reviews are prompt and contain deep insights on how to make code changes and the impact of those code changes. Another being it's a Google project after all LOL. I'd learn so much from how their developers write code, considering they hire the best of the best engineers/developers in the field. After assessing a couple of these benefits I decided to continue my journey with this project for 0.4 release.

Plan/Approach

As mentioned earlier, I wanted to build upon my knowledge for this project so I did just that. I did a big PR in 0.3 outlined here which was related to fixing linting issues for TypeScript (TS) code. That PR was a mix of linting plus barely scraping the surface of TS code where most of the ESLint warnings were about fixing JSDoc comments and just a mere 2 warnings were about fixing types in TS. Even though I didn't directly add or update the TS code in the project, I regularly interfaced with it during the 0.3's PR which gave me enough confidence to dive deeper into writing TS code for an issue. This was going to be the first time I contributed directly to writing TS code and coupling it with a project of a scale this big, so it was a big deal for me after all. Even though TS is a wrapper over JS we can give it the benefit of the doubt that it won't take that much of an effort from my side to write TS code compared to Go code let's say (because I know nothing about Go), however, that doesn't mean it'll be a smooth sailing ride for me just because TS and JS are brothers. Whatever it had to throw at me I was ready for it.

I started searching for a couple of issues within the project specifically related to TS. The idea was to do feature requests, a bit of a bigger outreach from my end towards the project rather than doing small PRs with good-first-issue tags. And that's when I stumbled upon these 2 issues namely Enable strict mode for workspace-backpack and Enable strict mode for workspace-search. After reading the description of issues it became clear that they fit well with the scale of the PR that I was looking for. Building upon my knowledge from the previous release, these 2 issues specifically targeted enabling strict mode for TS code within 2 plugins namely workspace-backpack -- allows a user to save current blocks on the workspace to a backpack and workspace-search -- allows a user to search for items present within the blocks on the current workspace.

As per TS documentation website a strict flag enables a wide range of type-checking behaviour that results in stronger guarantees of program correctness. Turning this on is equivalent to enabling all of the strict mode family options, which are outlined below:

noImplicitAny: disallows implicit any types.
strictNullChecks: disallows nullable values.
strictFunctionTypes: enforces stricter function types.
strictBindCallApply: enforces stricter bind, call, and apply methods.
strictPropertyInitialization: enforces stricter property initialization.
alwaysStrict: always emit use strict directive.

Both the issues mentioned, enabling strict mode for TS code would result in build time issues like adjusting types, adding checks for null or undefined, or making other fixes that needed to be addressed allowing me to directly write TS code, the first of two goals. Besides, this also allows me to penetrate deeper into the project's codebase which is the second of two goals for this release. I'm looking forward to successfully achieving both of these goals in this release.

Publishing package to npm registry, tagging and doing a release, and beyond...

Soham Thaker — Tue, 21 Nov 2023 01:32:46 +0000

Release

You can find my release here.

Release tool and package registry used

npm

Installing the package

Installation instructions for node-tilify package can be found here.

Process of creating a release

I watched a couple of videos, read the documentation and a blog before I was able to successfully release the tool on npm. Videos watched include Creating Node.js modules & Publishing npm packages from npm's youtube channel and How To Create And Publish Your First NPM Package from Web Dev Simplified's youtube channel. Reading documentation included npm's very own documentation and blog includes Publishing a Node.js CLI tool to npm in less than 15 minutes. The steps I performed are below:

Ran npm run adduser to create a user account on npm and link that user account locally to my machine. Ran npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with v1.0.0. Tested by running npm i node-tilify followed by node-tilify -h and it didn't work.
I assumed the location of the index.js was incorrect so I moved it from /src dir to / root dir part of commit b832ef9e. Ran npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with patch release v1.0.1. Tested by running npm i node-tilify followed by node-tilify -h and it didn't work.
I assumed the code within /index.js was exposed out in the open without being invoked by a function so I edited the code by putting all of the code into a function and then invoking that function at the bottom of /index.js part of commit 6597c56a. Ran npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with patch release v1.0.2. Tested by running npm i node-tilify followed by node-tilify -h and it didn't work.
According to the blog that I read, it suggested adding a shebang line #!/usr/bin/env node at the top of index.js file so that systems like Unix can understand what executable to use to run the script, which in this case is node. Also added a property "bin": "./index.js" within /package.json file so that npm can install the package globally, refer to the bin entry and link the package name with the index.js file as specified in the ./package.json configuration parth of commit 1583e704. Ran npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with patch release v1.0.3. Tested by running npm i node-tilify followed by node-tilify -h and it didn't work.
I missed one step from the blog, running npm link. npm link is a command-line tool for symlinking a local package as a dependency during development. It is commonly used for testing packages before publishing them. Ran npm link to link the package locally, followed by npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with patch release v1.0.4. Tested by running npm i node-tilify followed by node-tilify -h and it finally started working.
It was then time to update the documentation with latest information on how to install the package from npm. I pushed the commit e8eb1820. Ran npm version patch to upgrade the semver, followed by npm publish —access public to publish the package. Package was released with patch release v1.0.5. Tested by running npm i node-tilify followed by node-tilify -h and it worked fine.
Finally it was time to push all the tags to Github, ran git push --follow-tags command and then performed a release manually from GitHub to release v1.0.6 to the public.

User Testing

Manual user testing went quite well, which was unexpected. My tool only has 1 step to get it installed on another user's computer and they are ready to go to use the software. They did however point out a thing to fix on my documentation. The readme.md file mentioned the incorrect way to use the tool. It mentioned invoking the tool using node src/index.js -h which was now changed to node-tilify -h. Besides, they mentioned moving all the information regarding developer contributions like cloning a repo, installing the packages and running the repo code from the command line to contributing.md for better clarity. Also, they got stuck figuring out what passing a directory meant for input and output flags, so I explained it to them and they were able to get going after that point. Moreover, initially, they were reluctant to use toml configuration from the command line but then figured it out after reading the documentation a couple of times. Based on the feedback received the documentation was updated accordingly.

Learning Outcomes

This was a fantastic exercise for me considering I've never published packages to npm and releasing software to the public. I learnt how to test the software before publishing or releasing, publish a package on a registry, add git tags being part of the release, release the software on GitHub and perform user testing to gain valuable feedback from them to improve the documentation which in turn increases user adoption helping to capture wider audience who'd use the software.

Integrating CI Workflow in my project

Soham Thaker — Wed, 15 Nov 2023 00:20:50 +0000

Lab

I had to integrate GitHub Actions into my project this week to automatically run linting and unit test scripts whenever a new PR and/or a new push is added to my project.

Successful GitHub Actions Run

https://github.com/sdthaker/Node-TILify/actions/runs/6857717308

PR on another project's repo

https://github.com/Yousef-Majidi/Learn2Blog/pull/33

Feature Implementation

GitHub Actions can be used for building a piece of software or aid in deploying it. They are set up within a root folder called .github containing a folder called workflows. For example, if we take a look at the project I worked on, it has a lint-test.yml file located within /.github/workflows which defines a workflow of what it's supposed to do.

A workflow is a configurable automated process that will run one or more jobs. Workflows are defined by a YAML file checked into a repository which can be given a name, which in this case is name: Lint and Test and will run when triggered by an event in the repository, which in this case is,

on:
  push:
    branches: [ "master" ]
  pull_request:
    branches: [ "master" ]

A job is a set of steps in a workflow that is executed on the same runner. Each step is either a shell script that will be executed or an action that will be run. Steps are executed in order and are dependent on each other. When a new PR is created or a push is performed, this workflow will run with 2 jobs namely lint & test.

This was the entire workflow I added to the existing build workflow,

Breaking down the lint job:

lint:: This is the name of the job. It's used to identify the job in the GitHub Actions UI.
timeout-minutes: 5: This sets a timeout for the job. If the job runs for longer than 5 minutes, it will be automatically cancelled.
runs-on: ubuntu-latest: This specifies the type of runner that the job will run on. In this case, it's the latest version of Ubuntu.
steps:: This is a list of steps that the job will execute. Each step is an individual task that contributes to the job.
- uses: actions/checkout@v2: This step uses the checkout action at version 2 to checkout the repository's code onto the runner.
- name: Setup node: This step sets up a Node.js environment on the runner. It's given the name "Setup node" for clarity in the GitHub Actions UI.
- uses: actions/setup-node@v3: This uses the setup-node action at version 3 to set up the Node.js environment.
- with:: This is used to pass parameters to the setup-node action.
  - node-version: 18: This specifies that the Node.js version to be set up should be version 18.
- name: Npm Install: This step installs the npm packages required for the project. It's given the name "Npm Install" for clarity in the GitHub Actions UI.
- run: npm install: This runs the command npm install to install the npm packages.
- name: Lint: This step checks the format of the code. It's given the name "Lint" for clarity in the GitHub Actions UI.
- run: npm run lint: This runs the command npm run lint to check the format of the code.

Now that I integrated GitHub Actions in the repository, it was a pretty good exercise for me considering I've only done it once, helping me solidify my learnings from my previous contribution during 0.3 release. Automating tasks like build, test, lint, and format among other tasks using a CI workflow makes sure that key tasks are taken care of before the code gets deployed to the production because when running these tasks manually, developers tend to forget to run them. Besides, this improves DX and more and more developers would like to contribute to a project when you have this automation because mundane tasks like these are already taken care of.

Working in partnership

My partner's repo used an entirely different language/framework, one that I have never worked on before; Dot net. The instructions were quite simple to set up the project and to run tests and he also helped me with setting up dot net on my computer considering I have a Mac. Writing tests however were somewhat challenging considering I've never worked with C# but I used one of the other test file within the project as a reference to write test code and also to fix the errors. Besides, there was a weird bug I received while writing the test. The test I wrote checked whether the correct HTML string was returned by a function. The repo maintainer wrote this code on a Windows machine where the new lines are treated differently than those on Mac. After a bit of digging through my unit test code, I realized that this was the issue so to test the code I removed all the new lines from the code I didn't write and added them back after which it passed the test. I issued a PR and the pipeline automatically triggered, but considering this project was developed on Windows and I have a Mac, it gave the CRLF vs LF error for the string I wrote. I informed the repo owner about this and he said he'll fix this by removing the end lines and re-adding them from his machine. The PR is open at the time of writing this blog. This task gave me great exposure to work on another project which I didn't write along with a language/framework that I've never worked on before.

Delving into open source territory with Google: An Investigation

Soham Thaker — Fri, 10 Nov 2023 19:49:57 +0000

Project repo:

blockly-samples by Google Open Source.

Project description:

Blockly by Google Open Source is a JavaScript library for building visual programming editor, which looks something like this,

The project I worked on is called blockly-samples which is a supplementary project to the Blockly project. This project holds code for Plugins which are self-contained pieces of code that add functionality to Blockly. Plugins can add fields, define themes, create renderers, and much more, Examples which are self-contained sample projects demonstrating techniques on how to use Blockly's features and extend the Blockly library and Codelabs which are interactive tutorials demonstrating how to use and customize Blockly.

Issue

1923

PR

N/A

Code Fixes

For this issue, I didn't file a PR even though I was planning to do it originally but the dynamics of the issue changed after I started working on it. Even though I didn't file a PR, I still thought to include this issue as part of the 0.3 release because I spent time and effort on it to test the solution for project maintainers.

I worked on a plugin called workspace-minimap for this issue. You can find its codebase here and it's interactive plugin demo here. A workspace-minimap is a minimap that shows the entire workspace on a tiny map located at the top-right of the workspace. It provides improved navigation and organization within a workspace. A workspace-minimap looks like this,

Minimap mimics the entire workspace. Certain blocks on a workspace can have comments on them. For example function blocks,

When a user opens the comment section from a block present in the workspace and edits it, it won't update the minimap comments in real-time. So the user has to refresh the page to see the updated comments on minimap. Then when they click on the comments button on the blocks present on the minimap it'll show the comments but, they're in read-only mode, which I'm assuming they shouldn't be. Users should be allowed to edit the comments from the minimap as well, as per my understanding from the description attached on the issue. The project maintainers mentioned that the minimap isn't a perfect impression of the workspace so opening the comments should be disabled from minimap altogether.

Since I was new to the codebase, it was hard for me to figure out what they asked the contributor to do to fix the issue. I asked them to give me more information on how to disable the comments feature on minimap since I couldn't decipher the original explanation mentioned to resolve the issue. The description mentioned modify the minimap workspace config to set comments: false which didn't make sense to me since I looked for a config file that is possibly setting config options for the minimap, but that wasn’t the case.

They replied by pointing me to the perfect spot to look for and what to edit to fix the issue. And I went on a mission to fix this bug.

It was a simple fix, by adding comments: false to the workspace-minimap configuration object which helps build the minimap,

After adding the code changes, I re-ran the reproduction steps only to figure out that the changes I made didn't do anything. At which point I made a video explanation (will download a video file, upon clicking the link) to explain my investigation on this issue.

After a bit of testing on their end, the project maintainer replied saying
they were under the wrong impression of what these code changes do. These changes merely disable the users from creating/editing the comments and do not entirely disable comments from being constructed programmatically.

At that point, I concluded working on this issue by providing an ending remark since they wanted to run this issue through their triage team.

It was a good exercise for me regardless since my main goals from the 0.2 to 0.3 release were to interface more with code that is part of a larger project and facilitate healthy and fruitful conversations with the maintainers, among others as mentioned in first blog of this series within the section called "Measuring progress from 0.2 release".

Delving into open source territory with Google: PR #3

Soham Thaker — Fri, 10 Nov 2023 19:49:49 +0000

Project repo:

blockly-samples by Google Open Source.

Project description:

Blockly by Google Open Source is a JavaScript library for building visual programming editor, which looks something like this,

Issue

1979

PR

2060

Code Fixes

This issue was about re-enabling an ESLint - JSDoc rule named jsdoc/tag-lines which deals with enforcing a recommended blank line rule preceding only the first tag (e.g., @param) in a JSDoc comment. I reenabled the rule 'jsdoc/tag-lines': ['error', 'any', {'startLines': 1}], part of eslint.config.js file of the project. You can find more about this rule here.

Breakdown of the rule:

'jsdoc/tag-lines': This is the name of the rule. The jsdoc/tag-lines rule enforces consistent newlines before and after jsdoc tags.
['error', 'any', {'startLines': 1}]: This is the configuration for the rule.
- 'error': This sets the level of the rule. When ESLint encounters a violation of this rule, it will produce an error message. Other possible values are 'warn' (produces a warning message) and 'off' (turns off the rule).
- 'any': This is the first option for the rule. It means that the rule applies to any jsdoc tag.
- {'startLines': 1}: This is the second option for the rule. It means that there should be 1 newline before the start of the jsdoc tag lines.

An Example

Example of what the code changes look like after running the lint script,

Before:

/**
  * Parses XML based on the 'inputs' attribute (non-standard).
  * @param xmlElement XML storage element.
  */

After:

/**
  * Parses XML based on the 'inputs' attribute (non-standard).
  *
  * @param xmlElement XML storage element.
  */

And just like that, this PR was also handed in. Even though it was a small PR, I had to go through the eslint-plugin-jsdoc documentation to understand the purpose of this dependency, aligning with my goals on measuring my progress from 0.2 to 0.3 release. Besides, other things I focused on in this release are covered in first blog of this series within the section called "Measuring progress from 0.2 release".

Delving into open source territory with Google: PR #2

Soham Thaker — Fri, 10 Nov 2023 19:49:40 +0000

Project repo:

blockly-samples by Google Open Source.

Project description:

Blockly by Google Open Source is a JavaScript library for building visual programming editor, which looks something like this,

Issue

1977

PR

2060

Code Fixes

This issue was about adding a feature, specifically GitHub Actions workflow that checks for formatting errors and prints out a human-friendly message and a list of unformatted files. The script npm run format:check to run this task was already set, part of the package.json file, which in turn runs prettier --check .. However, I had to research a bit about what are GitHub Actions and how to set it up.

GitHub Actions can be used for building a piece of software or aid in deploying it. They are set up within a root folder called .github containing a folder called workflows. For example, if we take a look at the project I worked on, it has a build.yml file located within /.github/workflows which defines a workflow of what it's supposed to do. This workflow lints, builds and tests the repository.

A workflow is a configurable automated process that will run one or more jobs. Workflows are defined by a YAML file checked into a repository which can be given a name, which in this case is name: Build and Test and will run when triggered by an event in the repository, which in this case is on: [pull_request].

A job is a set of steps in a workflow that is executed on the same runner. Each step is either a shell script that will be executed or an action that will be run. Steps are executed in order and are dependent on each other. When a new PR is created this workflow will run with 3 jobs namely build, lint & format.

This was the job I added to the existing build workflow,

Breaking down the job:

format:: This is the name of the job. It's used to identify the job in the GitHub Actions UI.
timeout-minutes: 5: This sets a timeout for the job. If the job runs for longer than 5 minutes, it will be automatically cancelled.
runs-on: ubuntu-latest: This specifies the type of runner that the job will run on. In this case, it's the latest version of Ubuntu.
steps:: This is a list of steps that the job will execute. Each step is an individual task that contributes to the job.
- uses: actions/checkout@v2: This step uses the checkout action at version 2 to checkout the repository's code onto the runner.
- name: Setup node: This step sets up a Node.js environment on the runner. It's given the name "Setup node" for clarity in the GitHub Actions UI.
- uses: actions/setup-node@v3: This uses the setup-node action at version 3 to set up the Node.js environment.
- with:: This is used to pass parameters to the setup-node action.
  - node-version: 20: This specifies that the Node.js version to be set up should be version 20.
- name: Npm Install: This step installs the npm packages required for the project. It's given the name "Npm Install" for clarity in the GitHub Actions UI.
- run: npm install: This runs the command npm install to install the npm packages.
- name: Check Format: This step checks the format of the code. It's given the name "Check Format" for clarity in the GitHub Actions UI.
- run: npm run format:check: This runs the command npm run format:check to check the format of the code.

Besides, post issuing a PR, there were some small issues to take care of, like signing up for Google's Contributor License Agreement, following conventional commit specification for commit messages, rebasing the branch onto latest master due to out of sync packages which were failing the Node build and formatting the code.

This was my first time writing GitHub Actions. Its documentation helped me a lot in understanding and breaking down every task that I wrote part of this PR. The issue also mentioned looking at a similar job setup in another repository found here.

Besides, I have an upcoming lab to set up GitHub Actions on my project and learnings from this PR have already laid the foundation for that lab. Also, this is a proud moment for me knowing that every time someone is going to create a PR on this project, a code that I wrote on a “Google” project, will trigger a GitHub Action to check for their PR's formatting issues.

As far as gauging progress from 0.2 to 0.3 release is concerned, I promised myself to dive deeper into new avenues of software development and this PR is an ideal example of that. Besides, other things I focused on in this release are covered in first blog of this series within the section called "Measuring progress from 0.2 release".

Delving into open source territory with Google: PR #1

Soham Thaker — Fri, 10 Nov 2023 19:49:33 +0000

Project repo:

blockly-samples by Google Open Source.

Project description:

Blockly by Google Open Source is a JavaScript library for building visual programming editor, which looks something like this,

Issue

1978

PR

2065

Code Fixes

This issue I worked on for this PR was about fixing 16 ESLint warnings which show up upon running the script npm run lint from the root level of the project. Some of them were easy to fix, some not so much. The different warnings I fixed in this PR were,

Unexpected any. Specify a different type - @typescript-eslint/no-explicit-any
Missing JSDoc @param "type" type - jsdoc/require-param-type
Missing JSDoc comment - jsdoc/require-jsdoc
JSDoc @returns declaration present but return expression not available in function - jsdoc/require-returns-check
Syntax error in type: MigrationData[]
Missing JSDoc @returns type
Missing JSDoc @param "identifier" description - jsdoc/require-param-description
Missing JSDoc @param "identifier" type
'Minimap' is defined but never used - @typescript-eslint/no-unused-vars
JSDoc @returns declaration present but return expression not available in function - jsdoc/require-returns-check

Example: A couple of easy fixes

Example #1

One of the warnings I received on line 19 of blockly-samples/plugins/block-dynamic-connection/test/dynamic_if.mocha.js file was Missing JSDoc @param "type" type jsdoc/require-param-type. The code before the fix is below,

If we look closely, we see that the order of the parameter named type is jumbled up where the tag @param is supposed to come first, then comes the type of the parameter {string=} followed by the parameter name type. So this was quite an easy fix, all I had to do was move the parameter type to after the declaration of its type like this,

Example #2

Another warning I received was on line 134 of /blockly-samples/plugins/block-dynamic-connection/src/dynamic_list_create.ts file was Unexpected any. Specify a different type @typescript-eslint/no-explicit-any. The code before the fix is below,

Here the type of the parameter state is either an object or a string. The object would contain string for its key type and any type for its values, which ESLint was not happy about. It was somewhat hard for me to find out the type of the parameter because I tried to find the usage of this function and within that, a piece of code that set the parameter state to an object by assigning it a key-value pair, that could've given me the information about the type of the key's value, however, I couldn't find anything of that sort. Then I started reading the code written in the function and noticed that the value of state['itemCount'] was assigned to another variable called itemCount, like this,

I looked for itemCount declaration in this file and saw that it was initialized like this,

which immediately gave me the impression that the type of state's object's values would probably be a number because itemCount is initialized by a number and also the count in the name of the variable itemCount gives it up that it'll be of number type. Code after the fix was,

Example: A couple of hard fixes

Example #1

A warning I was trying to fix was on line 221 of /blockly-samples/plugins/field-bitmap/src/field-bitmap.js file about Missing JSDoc comment jsdoc/require-jsdoc. A jsdoc comment was missing for a function of FieldBitmap class and I had to add an explanation about the purpose of the function and its return value. Click here to interact with the FieldBitmap plugin. Visually, it looks like this,

The code before the fix is below,

So, I started reading and understanding the piece of code written within the function. The code itself wasn't self-explanatory because it was a very small piece of code and a quite technical one. So I used 2 approaches, one was to read and understand the purpose of the variable this.fieldGroup_, the function getBoundingClientRect() and the return value Blockly.utils.Rect(boundingBox.top, boundingBox.bottom, boundingBox.left, boundingBox.right,); and another approach was to give the Copilot Chat extension in VSCode the above code and make it explain what the code does. The Copilot couldn't help much initially when I gave it the above code so I started reading the code.

My hunt to understand the purpose of getScaledBBox() is detailed below:

To understand the purpose of the fieldGroup_, my hunt took me to Blockly's Field class file where the variable is declared on L125 with a type of SVGGElement | null since the getScaledBBox() function was part of a class called FieldBitmap and this class was extending Blockly's Field class implementation. Its declaration had a comment on top of it /** The rendered field's SVG group element. */ which gave me a better visualization and understanding of what this variable represents.

Next I went on a hunt to look for the function getBoundingClientRect() which I thought was a member function of SVGGElement class since it was being called on the fieldGroup_ variable. Alt + left clicking on the SVGGElement type annotation took me to an internal implementation of the class, part of TS's codebase and it had a comment on top of its declaration which took me to the class's documentation site. This was just an interface and didn't implement the getBoundingClientRect() function I was looking for so I went to its parent interface SVGGraphicsElement.

The class SVGGraphicsElement implemented SVGElement which in turn implemented Element which is where I finally found the getBoundingClientRect() implementation. The return value of this function is an SVG's group element's (an instance of DOMRect) properties like left, top, right, bottom, x, y, width, and height. These describe the position and size of the rectangle in pixels.

The top, right, bottom & left properties are used to re-create a Blockly Rect and then eventually gets returned from the function getScaledBBox().

This rectangle is used to create the field bitmap as we can see in the image attached above in the current section, showing what it looks like visually. The purpose of the field bitmap is that it lets users input a pixel grid with their mouse. They can paint over the pixels with their mouse by holding the left click and moving the cursor around the grid, or randomize the grid by clicking the Randomize button.

I combined all this information and gave it to the Copilot Chat extension which provided me with good enough information on which I can base my reference, to write the jsdoc comment for the function along with what I learnt through the above process, since at every step I found that the code either had a comment written on top of a declaration be it function or variable to briefly explain its purpose or found a proper documentation site explaining the purpose of the class, in this case mdn web docs, to help me understand what each piece of code means. I used the results from the Copilot and my understanding of the code to put together the jsdoc comment. The code after fix was,

This was quite a long process to fix this warning and took me quite a lot of time to get it right but I learnt a lot from it.

Example #2

Another warning that I was trying to fix was

for this code,

located in the file /blockly-samples/plugins/migration/bin/fix-imports.js part of keyboard-navigation plugin. Its interactive demo can be found here. The purpose of this plugin is to provide keyboard navigation for better accessibility through a keyboard cursor. You can find its documentation site here to learn more about it.

This warning is quite ambiguous. It doesn't explicitly mention what's the issue with the type, it just says the type is invalid. And this issue haunted me for a while to figure out what could possibly be an issue and in the end I had to put out a PR with the issue still unfixed. The issue hasn't been fixed since the PR is still open at the time of writing this blog and have asked the code maintainers to help me with this warning. I took help of AI and the code maintainers to find a fix for it but still couldn't fix it. There's a long discussion that occurred as you can notice on the filed issue #1978 with code maintainers to try different ways to fix this warning but in the end we decided to file a PR and take up the code changes as a follow-up during the PR review process. I will update the blog about the fix for this issue, if they review my PR and get back to me sooner.

Some of the tried approaches were:
Removing the tailing comma. The trailing comma is sometimes not allowed due a linting rule.

Changing the type definition, since the current type definition might be deprecated or has errors to something of this sort,
/**
* @typedef {Object} MigrationData
* @property {string} import
* @property {string|undefined} oldIdentifier
* @property {string} newIdentifier
* @property {string} newImport
* @property {string} newRequire
*/
Changing name and all its appearances of key named import to importName since import is a reserved keyword in JS/TS.

Optional declaration of key named oldIdentifier by changing it to oldIdentifier: string | undefined from oldIdentifier?: string since optional declaration might not be allowed with ESLint and JSDoc configuration.
Some of these approaches were suggested by the project maintainers and some were my own implementation based on my research but nothing worked. I also tried to use AI to help me fix this warning but that didn't work either.

N.B.

There's couple other warnings I fixed in this PR which were hard to deal with and took quite a long time for me to get through them which I haven't mentioned in this blog because I wanted to focus on the key fixes of this PR.

Code review and feedback

I received 4 code reviews on my PR. The reviews and the proposed changes are listed below,

Received warning: Syntax error in type: MigrationData[] on L62. Removed . in Array.<MigrationData> type declaration which was a quite simple fix i.e., to remove the dot. Array.<MigrationData> and Array<MigrationData> are different in syntax but are functionally equivalent. Using the dot is an older, non-standard way of declaring generic types so I had to get rid of is since the reviewer mentioned they don't use the dot syntax anywhere in the codebase.

Received warning: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any on L41. Removed explicit cast of a function from createWorkspace as any to
    createWorkspace as (
      blocklyDiv: HTMLElement,
      options: Blockly.BlocklyOptions,
    ) => Blockly.WorkspaceSvg
to simply passing it as createWorkspace in a function argument. The function is declared in the same file above, before it gets passed as an argument to another function and I failed to check that if the function is passed just by its identifier, removing the original as any cast, whether TS infers its type automatically or not. Reviewer mentioned that the full explicit cast is unnecessary and thus I removed it.

Received warning: JSDoc @returns declaration present but return expression not available in function jsdoc/require-returns-check on L448. Restored the changes from return done(); back to done(); return; within a function. The return type of the function was declared as Function so I assumed that I should be returning the done() function call. However, reviewer mentioned that returning done() is not the correct logic, instead asked to keep the previous logic as is and update the JSDoc comment to a return {Function | undefined} instead of {Function} which'll cover all possible types of return statements in this function.

Received warning: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any on L134. Updated changes for a function argument declaration from state: {[x: string]: any} | string to state: {[x: string]: number} | string to an updated declaration state: { itemCount?: number, [x: string]: unknown} | string. My assumption that state's keys will always be number was incorrect. Reviewer mentioned that this argument's keys could be of type number, boolean, string, etc, which I confirmed on their main project Blockly where it is assigned values with all sorts of types. So the ask was to add the key itemCount explicitly as an optional key of type number and the rest of the types for keys' values in this object will be changed from any to unknown to enforce strict type checking. After bit of research I came to know that the reason why unknown is strict because it enforces type safety; a developer before assigning the value of a variable declared as unknown to another variable, must explicitly check for a valid type and only then they can assign it to another variable. Besides when performing an assignment like this, this.itemCount = state['itemCount']; it gave an error since state's itemCount key is declared as optional which means the assignment must fallback to a default value when state doesn't have a key-value pair of itemCount. The reviewer suggested to add nullish coalescing like this this.itemCount = state['itemCount'] ?? 0; to account for cases like these.

Measuring progress from 0.2 release

In the 0.2 release during Hacktoberfest, my contributions primarily revolved around working on small code snippets like fixing the order of resolve, reject parameters, even tasks that did not involve coding like fixing transcript for a podcast and much smaller and simpler issues like updating a dependency but still added value to the projects. The projects I engaged with like Curio were relatively smaller in scale and utilizing a tech stack which I was already familiar with, specifically React.

Now, in the 0.3 release, I pushed myself to the next level. My contributions involved interfacing with a larger and more complex codebase for the project named blockly-samples. I've been extensively reading and understanding the code written by other developers to add my own contributions.

Furthermore, this time, I ventured into a project entirely written in TypeScript, a first for me, which is of great significance to me, as it aligns with my aspirations of becoming a web developer. This release has provided me with a valuable opportunity to learn TypeScript, a skill that I believe is essential in the ever-evolving field of web development.

One significant shift in my approach in the 0.3 release was the level of engagement with the project maintainers. I actively interacted with the maintainers, proposing changes, seeking clarification, and requesting their guidance when faced with challenges. This increased communication has not only expanded my knowledge but has also helped in building stronger relationships within the open source community.

Additionally, the 0.3 release was characterized by a significant emphasis on reading and understanding documentation. It involved a specific focus on reading the eslint-plugin-jsdoc documentation, which was essential for my work on fixing ESLint warnings for JSDoc. Besides, as part of my 0.3 release's contributions, 2 out 4 issues I worked on dealt with ESLint and I'd have to regularly visit its documentation site to get better understanding on working with ESLint. Moreover, in my contributions for 0.3 release, I extended my documentation exploration to core web APIs, through the use of well-documented MDN Web Docs. This shift reflects my commitment to building a deeper understanding of the tools and technologies I work with, enabling me to contribute more effectively.

A notable aspect of the 0.3 release was the extensive reliance on Document Object Model (DOM) manipulation due to the the nature of the project I chose to work on. To effectively contribute, I often had to refer to the Blockly's documentation. I regularly explored interactive examples for plugins & demos provided by the library helping me delve into the functionalities of various plugins to comprehend their role in the project. This experience strengthened my grasp of web development and enriched my understanding of how to create and manipulate elements in the DOM.

In summary, my 0.3 release stands out from the 0.2 release due to its increased complexity in terms of codebase, the introduction of TypeScript, more engagement with maintainers, and a deeper dive into documentation and DOM manipulation. I am excited about the opportunities for further growth and the contributions I can make in future releases.

Adding tests to my project

Soham Thaker — Tue, 07 Nov 2023 02:18:08 +0000

Commit

3dfea570

Lab

Tasks completed for this week's lab:

Set up a testing environment for my codebase using Jest.
Wrote a few tests to introduce unit testing for my codebase which tested the core of the codebase.
Added a test runner to watch for code changes for the code or test cases to re-rerun the tests and also allow the ability to simply run a single test, a single test suite or a single test file.
Updated documentation file, CONTRIBUTING.md to add usage of testing.
Added code coverage to check for how many lines of code is tested using the testing framework.

Setting up Jest

Set up Jest by installing it as a dev dependency using the command npm install --save-dev jest. Then I added tests directory to write my tests.
Installed 3 dependencies, babel/core, babel/preset-env & babel-jest to make sure it supports the import syntax in test files and added babel.config.cjs config file with options.
Added jest: true to the env object located in .eslintrc.cjs file for ESLint to recognize jest's global variables like describe, it, etc.
Added test script which in turn runs jest --.

Writing first Jest tests

I picked a very small function to test its input and output. The function reads & parses TOML files using a third-party library. In one test suite, I wrote four tests, one of them testing the try path and the other three testing the catch part. It was somewhat of a hiccup to get these tests to set up because the code in the catch block performed console.error() and process.exit() operations. So I had to read the API to find out how to test these operations. That's when I stumbled upon mock functions spyOn() & mockImplementation(). Mock functions are also known as "spies", because they let you spy on the behaviour of a function that is called indirectly by some other code, rather than only testing the output. So I spied on the 2 operations and was able to match the result provided by the function, with the result that I was expecting written in the test cases. A code example is below,

it('function should exit with error message & exit code -1 with incorrect toml file path', () => {
    const mockStdErr = jest
      .spyOn(global.console, 'error')
      .mockImplementation(() => {});
    const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {});
    readAndParseTomlConfig('file/path/that/doesnt/exist.toml');

    expect(mockStdErr).toHaveBeenCalledWith(
      'Error reading or parsing TOML file'
    );
    expect(mockExit).toHaveBeenCalledWith(-1);
  });

Allowing to run single tests and Test Runner Improvements

I added the capability to run a single file, test case or test suite using a single script npm run test:single blob. Here the blob argument can be a filename, test case or test suite. Also, I added the ability to have my test runner watch for changes and automatically run the tests when the test or source code is updated by using a single script npm run test:watch.

Test the Core

I added three tests part of the suite to test the core functionality of the project which is parsing MD to HTML. The three tests focus on testing the file I/O operations, specifically reading from a file, writing to a file and checking whether a file or directory exists. I added mock tests for it to simulate a real operation to test the functionality.

Adding code coverage analysis

Added code coverage analysis tool to test which files are tested and covered in testing and how much of the code is being missed out / tested on by running a single script npm run test:coverage.

Ignoring unnecessary folders

After running code coverage, it generates a folder named coverage. This folder is generated as a result of running a code coverage script. So there's no need to format, lint or commit the folder to GitHub. So I added the folder as part of .gitignore, .prettierignore & .eslintignore files.

Updating CONTRIBUTING.md

All of the above changes regarding tests have also been documented for ease of use.

Learning Outcomes

I understood the fundamental understanding of modern software testing practices by adding a test framework to my project. I learnt how to write concise and efficient test suites, create test cases for different parts of your codebase, and execute tests to ensure the functionality and reliability of my application. It also provided me with the skills needed to catch and address bugs early in the development process, leading to more robust and maintainable code since I was also fixing the code in the actual codebase while I was writing the tests for it. It gave me an understanding of advanced testing concepts like mocking, code coverage analysis, watching for test runner changes, etc. These skills will help me facilitate collaboration in a team, allowing me to be future-ready.

Adding code formatting, linting, pre-commit hooks and beyond...

Soham Thaker — Wed, 01 Nov 2023 15:28:08 +0000

Project

Node-TILify

Commit

7bf78de7

Lab

This week I had to set up and configure Prettier, ESLint, Husky & lint-staged dependencies to my codebase for better code formatting and linting and VSCode integration with prettier, eslint and pre-commit hooks using husky and lint-staged for achieving automation for linting and formatting.

What is Prettier?

It is a code formatting tool that helps developers format code as they like by setting up config files, for example, automatically converting all double quotes to single quotes.

How did I set up Prettier?

Prettier has pretty good documentation so I started with that. The steps involved in setting up Prettier were:

Added the prettier extension for VSCode.
Installed it as a dev dependency by running the command, npm install --save-dev --save-exact prettier.
Added .prettierrc file on the root level, with these overridden default settings,

{
  "tabWidth": 2,
  "useTabs": false,
  "bracketSameLine": false,
  "bracketSpacing": true,
  "embeddedLanguageFormatting": "auto",
  "endOfLine": "lf",
  "htmlWhitespaceSensitivity": "css",
  "proseWrap": "preserve",
  "printWidth": 80,
  "semi": true,
  "singleAttributePerLine": true,
  "singleQuote": true,
  "trailingComma": "es5"
}

Added .prettierignore file which ignores files/directories that are listed in it. For example, node_modules folder is not our code so we're better off not checking for code style in that folder. I ignored these files and folders in my project:

package.json
package-lock.json
node_modules/
examples/

Added an npm script to help users run prettier with a single command, npm run prettier which in turn runs the command prettier --write .

Besides, I formatted my code using this tool. It showed 2 errors. One was about the HTML's link tag not being closed. The fix was to put the href value in quotes and add a forward slash at the end of the tag. Also, it formatted all HTML files that my program generated which was pretty nice since I had trouble figuring out how to format those files within my code logic. Furthermore, I updated the CONTRIBUTING.md file with instructions on how to run the formatter.

What is ESLint?

It is a code linting tool that helps developers lint code as they like by setting up config files, for example, automatically adding a semi-colon after each line in a JS project.

How did I set up ESLint?

ESLint has pretty good documentation so I started with that. The steps involved in setting up ESLint were:

Added an eslint extension for VSCode.
Installed it as a dev dependency by running the command, npm install --save-dev eslint.
Added .eslintrc.cjs file on the root level, with these default settings,

module.exports = {
  env: {
    browser: true,
    es2021: true,
    node: true,
  },
  "extends": [
    "eslint:recommended",
    "prettier"
  ],
  parserOptions: {
    ecmaVersion: 'latest',
    sourceType: 'module',
  },
};

Added .eslintignore which ignores files/directories that are listed in it. For example, node_modules folder is not our code so we're better off not linting code in that folder. I ignored these files and folders in my project:

package.json
package-lock.json
node_modules/
examples/

Added an npm script to help users run eslint with a single command, npm run lint which in turn runs the command eslint .. It'll fix all the auto-fixable errors and pick the ones that need your attention.

Besides, I linted my code using this tool. It showed 2 errors. One was about process variables not being defined in a lot of files. process is a global variable easily accessible in Node.js project. ESLint wasn't aware of the language/environment that I was using in my project so I added node: true as part of env object in .eslintrc.cjs file to allow ESLint to import global vars automatically. Another error was about 2 redundant backslashes in a regex that I have in my code. I removed them and tested the code thoroughly to make sure everything was still operating properly. Furthermore, I updated the CONTRIBUTING.md file with instructions on how to run the linter.

Integrating ESLint with Prettier

As I was reading the Prettier documentation I stumbled upon this blob which talks about how to make ESLint and Prettier play nicely with each other. It instructed to install eslint-config-prettier dependency in my project using the command npm install --save-dev eslint-config-prettier. Then I added prettier as part of the .eslintrc.cjs' extends array making prettier dependency part of the linting process. Lastly, I ran its cmd line helper npx eslint-config-prettier path/to/main.js to check if there were any ESLint rules which are unnecessary or would conflict with Prettier. The test results came out fine with no unnecessary or conflicting rules. The use case on how it works and why to use it is mentioned here.

Integrating ESLint and Prettier into VSCode

I got the inspiration to add this integration task from my open-source course professor David and the telescope project. The purpose of this task was to add as much automation in the code formatting and linting as possible. I set up both tools in VSCode such that the code gets formatted and linted automatically when the user saves the file. I added .vscode directory on the root level, which helps VSCode read and override the default user settings to apply the settings listed in the files located in this directory. It contains 2 files, settings.json and extensions.json.

settings.json - This file sets VSCode's default formatter to prettier, enables format on save to beautify file formatting using prettier, enables code actions on save to fix all auto-fixable code errors using eslint, and automatically detects and uses the eslint configuration from the project's root directory. The JSON configuration object is added below,

{
  "editor.defaultFormatter": "esbenp.prettier-vscode",
  "editor.formatOnSave": true,
  "editor.codeActionsOnSave": {
    "source.fixAll": true
  },
  "eslint.workingDirectories": [{ "mode": "auto" }]
}

extensions.json — This file suggests a new contributor to your project to install eslint and prettier as extensions, the first time they open the project in the VSCode. The JSON configuration object is added below,

{
  "recommendations": [
    "dbaeumer.vscode-eslint",
    "esbenp.prettier-vscode",
  ]
}

What are Pre-commit hooks?

Hooks are essentially functions that hook into your git workflow and are triggered before or after a certain event is fired. For example, to automate the process of code formatting and linting, which was the sole reason to add this task to my project, we can set up hooks that would run just before a piece of code is committed in the git workflow. This ensures that the code being committed is consistent and free of errors which the human eye could miss.

How did I set up Pre-commit hooks?

I used 2 dependencies to set this up, namely Husky & lint-staged. Husky allows you to run hooks for every git-commit action whereas, lint-staged runs specified scripts when it matches staged files. The stages involved in this process were:

Installing and initializing husky using the command npx husky-init && npm install. It creates a .husky folder at the root level and creates a .husky/pre-commit hook shell script file which runs a command before each commit. One of the benefits of lint-staged is that it will run the commands that you define for the chosen staged files and also immediately add the changes to the staging phase if possible. It also allows you to run the linting and formatting in parallel to multiple staged files hence saving time when there are many staged files to commit.
Installing lint-staged and configuring it with Husky. Ran the command npm install --save-dev lint-staged to install the dependency and updated the npx test command in .husky/pre-commit to npx lint-staged. This allows Husky to run lint-staged which in turn runs the configuration file to perform linting and formatting operations.
Adding a .lintstagedrc.json file to set up linting and formatting commands. This file contains rules about which tools to run for which specific files. Also lint-staged automatically saves the changes performed during the formatting and linting operation before it adds the files to the commit stage. The rules set up for this tool were to run prettier and eslint (fix all auto-fixable issues) on javascript files and prettier on html and css files. The file contents are below,

{
  "*.js": ["prettier --write", "eslint --fix"],
  "*.{html, css}": ["prettier --write"]
}

Learning Outcomes

This was a wonderful experience for me to add all these tasks in my codebase. I always felt intimidated when it came to setting up these tasks using config files because I never integrated it in any of my previous projects. But time and time again the software development field makes me realize that the power of reading documentation is incredible. Almost all of the above integrations came from reading documentation for these tools.

Besides, developer experience (DX) is one of the key issues that everyone is trying to address these days. If the developers contributing to your open source project or developers in your team at work aren't happy then it'll affect code quality which is bad in general. One of the ways to improve DX is through code consistency, implemented using linting and formatting by following same rules across the project. Also, it helps with code readability & maintainability since it's much easier to read a piece of code written in same style across the board and it makes the code robust and bug resistant. Furthermore, having automation in your project allows for more and more developers joining your movement and contributing to it. Keep in mind, the more automation you have in your code the more it gets adopted and used.

Adding new feature to my project

Soham Thaker — Sun, 29 Oct 2023 16:18:09 +0000

Commit

b96ef38

Feature

The feature I picked was parsing images from markdown to HTML. In markdown, images are denoted as ![alt text](link to image). I already have support for 2 markdown features in my project, and now I extended it by adding support for image parsing. The reason to pick this feature was due to the already existing feature in my project which converts markdown links denoted as ![alt text](link to image) to HTML <a> tags. I looked at the Docusaurus documentation and they support link parsing which I already have in place. As I was going through the documentation I noticed they had static image parsing as well, which has subtle differences, where images in markdown are denoted as ![alt text](link to image) whereas links are denoted as [alt text](link). So, I thought this would be a simple, easy to implement and a great feature to add to my project.

Feature Implementation

From my findings, Docusaurus uses an external library to perform parsing of markdown images to HTML images. My assumption is that the external library uses regular expression to check if the phrase matches the pattern in the form of, ![alt text](link to image) but I could be wrong. Due to this assumption, I implemented my feature entirely using regex to parse images. Considering I already had code that parsed links, adding this feature was quite straightforward for me. My existing regex code that checked for links was /\[([^\]]+)\]\(([^\)]+)\)/g. After this feature
was added it got changed to /(\!?)\[([^\]]+)\]\(([^\)]+)\)/g. Here the first group captures 0 or 1 of the escaped exclamation mark, the second group captures the text for the image or link and the third group captures the link/path to the image or hyperlink. The regex can match patterns throughout the line that is being read using the global flag. The code that handles parsing for hyperlinks and images is below,

const mdBlobs = bodyArr[i].match(new RegExp(mdBlobRegex, 'g'));

        if (mdBlobs && mdBlobs.length > 0) {
          mdBlobs.forEach((blob) => {

            const blobGroup = blob.match(mdBlobRegex);

            if (blobGroup) {
              const [mdPhrase, mdDelimiter, mdAltText, mdURL] 
              = blobGroup;

              if (mdDelimiter === '!') {
                // Replace md image with <img> tags
                bodyArr[i] = bodyArr[i].replace(
                  mdPhrase,
                  `<img src="${mdURL}" alt="${mdAltText}" width=150 height=150 >`
                );
              } else {
                // Replace md links with <a> tags
                bodyArr[i] = bodyArr[i].replace(
                  mdPhrase,
                  `<a href="${mdURL}" target="_blank">${mdAltText}</a>`
                );
              }
          }
});

The above code first extracts all the hyperlinks and images from a line in markdown. Then it iterates over each of these blobs to extract the different parts of the blob grouped using the () notation in a regex. It gives details like the entire phrase/blob, delimiter like an exclamation mark, the alternate text and the link/path itself. Then it checks whether the delimiter which in the case of an image is an exclamation mark and replaces the entire phrase/blob with an image or anchor tag along with adding properties like src, alt, href, etc.

As far as testing is concerned, I tested my regex using a tool called regexr which breaks down a regex really well to explain each part of regex and also gives a playground to test the strings. Besides, I added multiple test cases in the markdown file where I'd put images with hyperlinks in the same line, images at the beginning, end and middle of a line, and a line full of quite a few images. The tool parsed the images and hyperlinks with no issues.

Next Steps

At the time of writing the blog, I haven't come across any issues that I can file for this feature but I'll actively be looking to find any issues that I could add with a detailed explanation to get people involved in my project. Moreover, I'll be using different tags like good first issue, beginner, difficulty:easy, bug, beginner-friendly, etc to get more eyes on my project. Also, I'm continuously finding ways to make my readme file as up-to-date and full of information as possible to attract more people. Besides, I aim to be there for help and support anytime a contributor needs me, with a good enough turnaround time. Furthermore, I'm aiming to add contributors to the dedicated contributors section or add a special contributors markdown file which mentions their names and GitHub handles, as a token to thank them for their contributions.