Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

Convert to Typescript 3.7 and ESLint #467

Merged
merged 13 commits into from
Nov 14, 2019
Merged

Convert to Typescript 3.7 and ESLint #467

merged 13 commits into from
Nov 14, 2019

Conversation

stevenvergenz
Copy link
Contributor

  • Update Typescript dependency to version 3.7.
  • Replace the deprecated tslint with the new eslint.
    • Port existing tslint rules to eslint (to the extent possible).
  • Move some NPM tasks to VSCode tasks (no loss in NPM functionality).

Copy link
Contributor

@eanders-ms eanders-ms left a comment

Choose a reason for hiding this comment

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

Looks good! 👍👍👍
A few minor comments.

@@ -62,6 +62,7 @@ export default class StatsTest extends Test {
const plainStats = stats as any;
let pp = '';
for (const k in plainStats) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These three lines could be simplified to

for (const k of plainStats)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -39,7 +39,7 @@ export default class UserTest extends Test {
private formatProperties(props: { [key: string]: string }): string {
let output = "";
for (const k in props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also could be simplified to

for (const v of props)

@@ -15,7 +15,7 @@ export default function readPath(src: any, dst: any, ...path: string[]) {
field = path.shift();
validateJsonFieldName(field);
if (path.length) {
if (!dst.hasOwnProperty(field)) {
if (!Object.prototype.hasOwnProperty.call(dst, field)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, what was the issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hasOwnProperty could be an assigned property of the object, instead of being the built-in method.

@stevenvergenz stevenvergenz merged commit 266cc3a into red Nov 14, 2019
@stevenvergenz stevenvergenz deleted the stvergen/ts-3.7 branch November 14, 2019 01:20
@eanders-ms
Copy link
Contributor

  this.leave = this.leave.bind(this);

Curious why this change. This is a common JavaScript code pattern. Was lint flagging it?


Refers to: packages/sdk/src/adapters/multipeer/client.ts:61 in a304995. [](commit_id = a304995, deletion_comment = True)

@eanders-ms
Copy link
Contributor

  this.leave = this.leave.bind(this);

What I was expecting: Convert leave to an arrow-method on the class. This should make it unnecessary to call bind.


In reply to: 554121429 [](ancestors = 554121429)


Refers to: packages/sdk/src/adapters/multipeer/client.ts:61 in a304995. [](commit_id = a304995, deletion_comment = True)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants