Skip to content

Commit

Permalink
chore: upgrade deno_core (#21036)
Browse files Browse the repository at this point in the history 
Updated to deno_core 0.224.0 and V8 12.0.

---------

Co-authored-by: Aapo Alasuutari <aapo.alasuutari@gmail.com>
  • Loading branch information
@bartlomieju @aapoalas
bartlomieju and aapoalas authored Nov 1, 2023
1 parent 01d3e0f commit 1d19b10
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 22 deletions.
20 changes: 10 additions & 10 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ license = "MIT"
repository = "https://github.com/denoland/deno"

[workspace.dependencies]
deno_ast = { version = "0.31.0", features = ["transpiling"] }
deno_core = { version = "0.223.0" }
deno_ast = { version = "0.31.2", features = ["transpiling"] }
deno_core = { version = "0.224.0" }

deno_runtime = { version = "0.129.0", path = "./runtime" }
napi_sym = { version = "0.51.0", path = "./cli/napi/sym" }
Expand Down
10 changes: 6 additions & 4 deletions cli/tests/node_compat/test/parallel/test-buffer-alloc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ const assert = require('assert');

const SlowBuffer = require('buffer').SlowBuffer;

// TODO(bartlomieju): this test started failing after update to V8 12.0,
// maybe the size limit was increased?
// Verify the maximum Uint8Array size. There is no concrete limit by spec. The
// internal limits should be updated if this fails.
assert.throws(
() => new Uint8Array(2 ** 32 + 1),
{ message: 'Invalid typed array length: 4294967297' }
);
// assert.throws(
// () => new Uint8Array(2 ** 32 + 1),
// { message: 'Invalid typed array length: 4294967297' }
// );

const b = Buffer.allocUnsafe(1024);
assert.strictEqual(b.length, 1024);
Expand Down
15 changes: 14 additions & 1 deletion cli/tests/unit/globals_test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
// deno-lint-ignore-file no-window-prefix
import { assert, assertEquals } from "./test_util.ts";
import { assert, assertEquals, assertRejects } from "./test_util.ts";

Deno.test(function globalThisExists() {
assert(globalThis != null);
Expand Down Expand Up @@ -140,3 +140,16 @@ Deno.test(function windowNameIsDefined() {
assertEquals(window.name, "");
assertEquals(name, "");
});

Deno.test(async function promiseWithResolvers() {
{
const { promise, resolve } = Promise.withResolvers();
resolve(true);
assert(await promise);
}
{
const { promise, reject } = Promise.withResolvers();
reject(new Error("boom!"));
await assertRejects(() => promise, Error, "boom!");
}
});
6 changes: 6 additions & 0 deletions cli/tsc/dts/lib.es2021.promise.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,10 @@ interface PromiseConstructor {
* @returns A new Promise.
*/
any<T>(values: Iterable<T | PromiseLike<T>>): Promise<Awaited<T>>

/**
* Creates a Promise that can be resolved or rejected using provided functions.
* @returns An object containing `promise` promise object, `resolve` and `reject` functions.
*/
withResolvers<T>(): { promise: Promise<T>, resolve: (value?: T | PromiseLike<T>) => void, reject: (reason?: any) => void };

This comment has been minimized.

Sign in to view

Copy link
@jsejcksn

jsejcksn Nov 3, 2023 •
edited
Loading

Contributor

The promise generic type is T, yet — unconditionally — the value parameter of the resolve function is optional (and therefore its union includes undefined). This is a contradiction which causes a type-safety issue (demonstrated below). If marking value as optional wasn't an accident, it would be better to conditionally mark it as optional only when T can be extended by void or undefined.

example.ts:

const { promise, resolve } = Promise.withResolvers<string>();
resolve();
//      ^ No string here, but compiles!!
const result = await promise;
//    ^? const result: string
console.log(result.toUpperCase());
% deno --version
deno 1.38.0 (release, aarch64-apple-darwin)
v8 12.0.267.1
typescript 5.2.2

% deno check example.ts
Check file:///Users/deno/example.ts

% echo $?
0

% deno run example.ts 
error: Uncaught TypeError: Cannot read properties of undefined (reading 'toUpperCase')
console.log(result.toUpperCase());
                   ^
    at file:///Users/deno/example.ts:6:20

The reference TypeScript implementation that's linked to by the proposal looks like this:

export interface Deferred<T> {
  resolve: (value: T | PromiseLike<T>) => void;
  reject: (reason: unknown) => void;
  promise: Promise<T>;
}

export function defer<T = void>(): Deferred<T> {
  let resolve!: (value: T | PromiseLike<T>) => void;
  let reject!: (reason: unknown) => void;
  const promise = new Promise<T>((_resolve, _reject) => {
    resolve = _resolve;
    reject = _reject;
  });
  return { resolve, reject, promise };
}

And the types can also be defined by derivation from the Promise constructor callback function:

Code in TS Playground

declare global {
  interface PromiseConstructor {
    withResolvers<T>(): {
      promise: Promise<T>;
      resolve: Parameters<ConstructorParameters<typeof Promise<T>>[0]>[0];
      reject: Parameters<ConstructorParameters<typeof Promise<T>>[0]>[1];
    };
  }
}

const { promise, resolve } = Promise.withResolvers<string>();
resolve(); /*
~~~~~~~~~
Expected 1 arguments, but got 0. (2554)
*/

This comment has been minimized.

Sign in to view

Copy link
@jsejcksn

jsejcksn Nov 3, 2023

Contributor

cc @bartlomieju

This comment has been minimized.

Sign in to view

Copy link
@lucacasonato

lucacasonato Nov 3, 2023

Member

@jsejcksn That sounds right. Do you mind opening a PR?

This comment has been minimized.

Sign in to view

Copy link
@jsejcksn

jsejcksn Nov 4, 2023

Contributor

@lucacasonato Created: #21085
}
11 changes: 6 additions & 5 deletions test_ffi/tests/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,12 @@ assertEquals(isNullBuffer(new Uint8Array()), false, "isNullBuffer(new Uint8Array

// Externally backed ArrayBuffer has a non-null data pointer, even though its length is zero.
const externalZeroBuffer = new Uint8Array(Deno.UnsafePointerView.getArrayBuffer(ptr0, 0));
// However: V8 Fast calls get null pointers for zero-sized buffers.
assertEquals(isNullBuffer(externalZeroBuffer), true, "isNullBuffer(externalZeroBuffer) !== true");
// Also: V8's `Local<ArrayBuffer>->Data()` method returns null pointers for zero-sized buffers.
// Using `Local<ArrayBuffer>->GetBackingStore()->Data()` would give the original pointer.
assertEquals(isNullBufferDeopt(externalZeroBuffer), true, "isNullBufferDeopt(externalZeroBuffer) !== true");
// V8 Fast calls used to get null pointers for all zero-sized buffers no matter their external backing.
assertEquals(isNullBuffer(externalZeroBuffer), false, "isNullBuffer(externalZeroBuffer) !== false");
// V8's `Local<ArrayBuffer>->Data()` method also used to similarly return null pointers for all
// zero-sized buffers which would not match what `Local<ArrayBuffer>->GetBackingStore()->Data()`
// API returned. These issues have been fixed in https://bugs.chromium.org/p/v8/issues/detail?id=13488.
assertEquals(isNullBufferDeopt(externalZeroBuffer), false, "isNullBufferDeopt(externalZeroBuffer) !== false");

// The same pointer with a non-zero byte length for the buffer will return non-null pointers in
// both Fast call and V8 API calls.
Expand Down

0 comments on commit 1d19b10

Please sign in to comment.