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

chore(NODE-5446): update driver v6 dependencies #3801

Merged
merged 6 commits into from
Aug 16, 2023
Merged

Conversation

W-A-James
Copy link
Contributor

@W-A-James W-A-James commented Aug 8, 2023

Description

What is changing?

Update the following dev dependencies

  • "@microsoft/api-extractor": "^7.36.4"
  • "@octokit/core": "^4.2.4"
  • "@types/node": "^20.4.8"
  • "@types/sinon": "^10.0.16"
  • "@typescript-eslint/eslint-plugin": "^5.62.0"
  • "@typescript-eslint/parser": "^5.62.0"
  • "eslint": "^8.46.0"
  • "eslint-config-prettier": "^8.10.0"
  • "eslint-plugin-import": "^2.28.0"
  • "gcp-metadata": "^5.3.0"
  • "semver": "^7.5.4"
  • "sinon": "^15.2.0"
  • "v8-heapsnapshot": "^1.3.1"
Is there new documentation needed for these changes?

No

What is the motivation for this change?

NODE-5446; keeping tooling up to date

Release Highlight

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@W-A-James W-A-James marked this pull request as ready for review August 9, 2023 19:13
@W-A-James W-A-James changed the title chore(NODE-5446): update driver v6 deps chore(NODE-5446): update driver v6 dependencies Aug 9, 2023
@baileympearson baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Aug 9, 2023
@baileympearson baileympearson self-assigned this Aug 9, 2023
Copy link
Contributor

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

question: how come we decided to disable ban-types in most of our lint configurations?

"eslint-plugin-import": "^2.27.5",
"eslint": "^8.46.0",
"eslint-config-prettier": "^8.10.0",
"eslint-plugin-import": "^2.28.0",
"eslint-plugin-prettier": "^4.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

did we decide not to bump gcp-metadata?

Copy link
Contributor Author

@W-A-James W-A-James Aug 14, 2023

Choose a reason for hiding this comment

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

Oh, my bad, I missed updating that one.

@W-A-James
Copy link
Contributor Author

@baileympearson re disabling ban-types, when updating the eslint and associated dev dependencies, we start getting the following lint failures

/home/wajames/node_driver/src/operations/operation.ts
  20:47  error  Don't use `Function` as a type. The `Function` type accepts any function-like value.
It provides no type safety when calling the function, which can be a common source of bugs.
It also accepts things like class declarations, which will throw at runtime as they will not be called with `new`.
If you are expecting the function to accept certain arguments, you should explicitly define the function shape  @typescript-eslint/ban-types

/home/wajames/node_driver/test/types/basic_schema.test-d.ts
  59:14  error  Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `object` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.
- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead  @typescript-eslint/ban-types

/home/wajames/node_driver/test/types/community/changes_from_36.test-d.ts
  16:15  error  Don't use `Function` as a type. The `Function` type accepts any function-like value.
It provides no type safety when calling the function, which can be a common source of bugs.
It also accepts things like class declarations, which will throw at runtime as they will not be called with `new`.
If you are expecting the function to accept certain arguments, you should explicitly define the function shape                                  @typescript-eslint/ban-types
  24:15  error  Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `object` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.
- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead  @typescript-eslint/ban-types

X 4 problems (4 errors, 0 warnings)

I wasn't sure if we wanted to address those here with code changes, so I disabled that rule, which in retrospect might have been a reach too far. I can disable the rule on a per-line basis and file a follow-up ticket.

.eslintrc.json Outdated Show resolved Hide resolved
@baileympearson baileympearson merged commit fd9a467 into main Aug 16, 2023
1 check passed
@baileympearson baileympearson deleted the NODE-5446 branch August 16, 2023 14:52
baileympearson added a commit that referenced this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants