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

fix(types): correct TypeCast's Next callback to return unknown #3129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bbugh
Copy link

@bbugh bbugh commented Oct 17, 2024

Fixes "typeCast Next type appears to be incorrect" #3122

Also replaced references to the old incorrect type signature in referencing comments

Fixes typeCast Next type appears to be incorrect sidorares#3122
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.13%. Comparing base (b92db2a) to head (5c22cf0).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3129   +/-   ##
=======================================
  Coverage   88.13%   88.13%           
=======================================
  Files          71       71           
  Lines       12890    12890           
  Branches     1355     1354    -1     
=======================================
  Hits        11361    11361           
  Misses       1529     1529           
Flag Coverage Δ
compression-0 88.13% <ø> (ø)
compression-1 88.13% <ø> (ø)
tls-0 87.55% <ø> (ø)
tls-1 87.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@wellwelwel wellwelwel left a comment

Choose a reason for hiding this comment

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

Thanks, @bbugh.

I've suggested some rollback changes to the website's test files, or if you prefer, the CI error is due to the last EOL character in ./website/test/fixtures/external-code-embed/QueryOptions.txt 🤝

@@ -32,7 +32,7 @@ export interface QueryOptions {
* Determines if column values should be converted to native JavaScript types. It is not recommended (and may go away / change in the future)
* to disable type casting, but you can currently do so on either the connection or query level. (Default: true)
*
* You can also specify a function (field: any, next: () => void) => {} to do the type casting yourself.
* You can also specify a function (field: any, next: () => unknown) => {} to do the type casting yourself.
Copy link
Collaborator

@wellwelwel wellwelwel Oct 18, 2024

Choose a reason for hiding this comment

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

No need to change the test fixtures or resources in ./website/test. They just ensure string extractions in the ExternalCodeEmbed tests and are for internal use only 🙋🏻‍♂️

@@ -41,7 +41,7 @@ export interface QueryOptions {
* Determines if column values should be converted to native JavaScript types. It is not recommended (and may go away / change in the future)
* to disable type casting, but you can currently do so on either the connection or query level. (Default: true)
*
* You can also specify a function (field: any, next: () => void) => {} to do the type casting yourself.
* You can also specify a function (field: any, next: () => unknown) => {} to do the type casting yourself.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@wellwelwel wellwelwel linked an issue Oct 18, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

typeCast Next type appears to be incorrect
2 participants