Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: #158 remove no-this-alias ignore eslint rule #160

Merged
merged 23 commits into from
May 1, 2024

Conversation

miquelbeltran
Copy link
Contributor

@miquelbeltran miquelbeltran commented Apr 25, 2024

refactor: #158 remove no-this-alias ignore from eslint rule.

Description 📝

More context on this lint rule: https://typescript-eslint.io/rules/no-this-alias/

Assigning a variable to this instead of properly using arrow lambdas may be a symptom of pre-ES6 practices or not managing scope well.

Type of change

  • tech debt

Updates

  • Removed ignored rule from config file.
  • Use of arrow lambdas to keep the constantly bound this reference (more info: https://stackoverflow.com/a/56206672/673294)
  • In the case of HttpOptions, the object is created before returning the MessageTransport object.

Test plan 🧪

  • Run the npm ci, npm test and npm run eslint commands to ensure no issues raised.
  • Verified that there were test cases using those changed functions.
  • Run examples.

Author to check 👓

  • Project and all contained modules builds successfully
  • Self-/dev-tested
  • Unit/UI/Automation/Integration tests provided where applicable
  • Code is written to standards
  • Appropriate documentation written (code comments, internal docs)

Reviewer to check ✔️

  • Project and all contained modules builds successfully
  • Change has been dev-/reviewer-tested, where possible
  • Unit/UI/Automation/Integration tests provided where applicable
  • Code is written to standards
  • Appropriate documentation written (code comments, internal docs)

@miquelbeltran miquelbeltran marked this pull request as ready for review April 29, 2024 06:19
http: {
host: client._host,
port: client._port,
useSSL: !!client._useSSL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_useSSL was doing this double negation (!!) to convert any null values into false, so I changed that to _useSSL || false as it looks more clear.

Copy link
Collaborator

@sumitramanga sumitramanga left a comment

Choose a reason for hiding this comment

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

These changes look reasonable. We are moving away from legacy code to the modern ES6 approach. LGTM! 🌜

@miquelbeltran
Copy link
Contributor Author

Waiting for #159 for merge

Base automatically changed from 158-eslint-fixes to develop May 1, 2024 05:20
@miquelbeltran miquelbeltran merged commit aea993f into develop May 1, 2024
4 checks passed
@miquelbeltran miquelbeltran deleted the 158-no-this-alias branch May 1, 2024 05:28
miquelbeltran added a commit that referenced this pull request May 6, 2024
* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* update package lock in sample
miquelbeltran added a commit that referenced this pull request May 8, 2024
* ci: Create node.js.yml (#142)

* ci: Create node.js.yml

* Include "develop" in the branch list to run CI actions

* trigger GitHub actions.

* fix: Updating dependencies and fixes (#144)

* chore: updated dependencies

* chore: update to tslint-react 5.0.0

* fix: mixup between Request and RequestParams

* fix: convert Express Request to RequestParams

* fix: recreate package-lock to fix express dependency errors

* fix: test tap command update

* fix: test asserts

* refactor: use equal instead of ok

* Adding PR, bug report and feature request templates.

* Update pull_request_template.md

Committing @sumitramanga's suggestion

Co-authored-by: Sumitra Manga <[email protected]>

* Update .github/bug_report.md

Nice!

Co-authored-by: Sumitra Manga <[email protected]>

* Update .github/bug_report.md

Co-authored-by: Sumitra Manga <[email protected]>

* ci: #145 eslint setup (#156)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* chore: #148 upgrade examples (#157)

* express example: upgrade dependencies and point to subfolder raygun package

* using-domains: upgrade dependencies and reference local raygun

* using-domains: upgrade dependencies and reference local raygun

* add console logs and format using spaces

* improved readme

* improved readme + replace tabs by spaces

* enforce changing API keys to run examples

* ci: add PR title checks (#167)

* ci: Update dependabot.yml (#168)

* chore(deps): bump actions/checkout from 3 to 4 (#171)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump actions/setup-node from 3 to 4 (#172)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 3 to 4.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v3...v4)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: Move pr.yml to workflows (#175)

* refactor: #158 remove no-explicit-any ignore eslint rule (#159)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* update package lock in sample

* Update lib/raygun.sync.worker.ts

Co-authored-by: Sumitra Manga <[email protected]>

* remove empty line

---------

Co-authored-by: Sumitra Manga <[email protected]>

* refactor: #158 remove no-this-alias ignore eslint rule (#160)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* update package lock in sample

* refactor: #158 fix eslint rule "no-unused-vars" (#170)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* ci: #174 Prettier setup (#169)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* Update .github/workflows/node.js.yml

Co-authored-by: Sumitra Manga <[email protected]>

* fix workflow

* add comments to the package.json

* add missing files

* remove fetch-depth: 0

---------

Co-authored-by: Sumitra Manga <[email protected]>

* chore: #158 document why no-var-requires ignore is necessary (#173)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* chore: document why no-var-requires ignore is necessary

update comment

* refactor: #158 configure no-undef eslint rule  (#176)

* removed tslint

* add eslint

* add default config

* setup default config and npm run command

* add command to node.js.yml

* set rules to support current codebase

* remove unused tslint.json

* include sample files in eslint command

* remove rule no-explicit-any

* specify type for timer

* remove any from filterKeys

* explictly declare that error can be Error, String but still accept any from the end users

* specify types in the sync worker

* improve the IndexableError type

* explain why any is allowed in CustomData

* remove no-this-alias from eslint config

* convert to arrow function to preserve this reference

* use arrow functions to remove the need of a this alias

* define httpoptions object instead of using this alias

* add examples to prettier command

* add prettier job to node js ci job

* script fix

* setup no-unused-vars lint rule

* remove unused var

* fixed lib/raygun.batch.ts

* fix lib/raygun.offline.ts

* cleanup lib/raygun.sync.transport.ts

* fix lib/raygun.transport.ts

* fix lib/raygun.ts

* fix @typescript-eslint/no-unused-vars

* fixs in test files

* update package lock in sample

* cleanup eslint config

* config files

* run npm run prettier

* chore: document why no-var-requires ignore is necessary

update comment

* fix: #158 configure no-undef eslint rule

* refactor: #158 configure no-unreachable eslint rule (#177)

* refactor: #158 remove no-useless-catch eslint rule (#178)

* refactor: #158 configure no-unreachable eslint rule

* refactor: #158 remove no-useless-catch eslint rule

* chore: Prepare for Release 0.14.0 (#179)

* refactor: #158 configure no-unreachable eslint rule

* refactor: #158 remove no-useless-catch eslint rule

* set version 0.14.0

* update changelog

* Update package.json

Co-authored-by: Sumitra Manga <[email protected]>

* add Error reporting to keywords

* update license copyright year

* updated URL

* added more keywords

* moar keywords

---------

Co-authored-by: Sumitra Manga <[email protected]>

* chore: Update package-lock.json (#180)

* chore: update package-lock.json (#183)

* chore: change author name (#185)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kai Koenig <[email protected]>
Co-authored-by: Sumitra Manga <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants