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

mod.ts: check for appropriate v8 version #455

Closed

Conversation

montyanderson
Copy link

We run deno-postgres for some of our API services. During a recent upgrade we had query failures due to no support for Promise.withResolvers(). This change adds a check for at least V8 version 12 where that API was added.

We run `deno-postgres` for some of our API services. During a recent upgrade we had query failures due to no support for `Promise.withResolvers()`. This change adds a check for at least V8 version 12 where that API was added.
@groksrc
Copy link
Contributor

groksrc commented Feb 8, 2024

Fwiw, I also ran across this error today and can confirm its existence.

mod.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@bombillazo bombillazo left a comment

Choose a reason for hiding this comment

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

Simplify condition

mod.ts Outdated Show resolved Hide resolved
Co-authored-by: Asher Gomez <[email protected]>
Copy link
Contributor

@langpavel langpavel left a comment

Choose a reason for hiding this comment

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

  • ❌ Comment is misleading. Deno.version.v8 in comment vs Deno.version.deno in code
  • ❌ Really you want compare string with float converted to string?

Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Please run deno fmt and see the above suggestions.

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.

5 participants