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

AbortSignal.reason not available in lib.dom.d.ts #14344

Closed
jaydenseric opened this issue Apr 21, 2022 · 15 comments · Fixed by #14430
Closed

AbortSignal.reason not available in lib.dom.d.ts #14344

jaydenseric opened this issue Apr 21, 2022 · 15 comments · Fixed by #14430
Assignees

Comments

@jaydenseric
Copy link

jaydenseric commented Apr 21, 2022

Something has changed in deno v1.21.0, as upgrading from v1.20.6 has caused TS errors to fail tests that used to run fine:

deno test \
  --unstable \
  --allow-env \
  --allow-net \
  --allow-run \
  --allow-read \
  --allow-write \
  --import-map=importMap.json
Check file://[redacted]/ruck/Effect.test.mjs
Check file://[redacted]/ruck/HeadManager.test.mjs
Check file://[redacted]/ruck/HeadManagerContext.test.mjs
Check file://[redacted]/ruck/Html.test.mjs
Check file://[redacted]/ruck/LinkCss.test.mjs
Check file://[redacted]/ruck/NavigateContext.test.mjs
Check file://[redacted]/ruck/RouteContext.test.mjs
Check file://[redacted]/ruck/TransferContext.test.mjs
Check file://[redacted]/ruck/assertImportMap.test.mjs
Check file://[redacted]/ruck/createPseudoNode.test.mjs
Check file://[redacted]/ruck/documentHasStyleSheet.test.mjs
Check file://[redacted]/ruck/hydrate.test.mjs
Check file://[redacted]/ruck/publicFileResponse.test.mjs
Check file://[redacted]/ruck/readImportMapFile.test.mjs
Check file://[redacted]/ruck/routePlanForContentWithCss.test.mjs
Check file://[redacted]/ruck/scrollToHash.test.mjs
Check file://[redacted]/ruck/serve.test.mjs
Check file://[redacted]/ruck/useOnClickRouteLink.test.mjs
error: TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
    return Promise.reject(createAbortError(signal.reason));
                                                  ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:27:51

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
  const abort = () => waiter.reject(createAbortError(signal.reason));
                                                            ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:30:61

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
    throw createAbortError(signal.reason);
                                  ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:46:35

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
  const abort = () => waiter.reject(createAbortError(signal.reason));
                                                            ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:49:61

Found 4 errors.

That output implies there is some kind of type check issue in useOnClickRouteLink.test.mjs, but the Deno LSP doesn't surface any in VS Code when looking at the module, and according to the new deno check command there are no type check issues with it:

deno check --import-map=importMap.json useOnClickRouteLink.test.mjs
Check file://[redacted]/ruck/useOnClickRouteLink.test.mjs
@lucacasonato
Copy link
Member

You are using the dom tslib which does not yet have typings for AbortSignal.reason. This is also the case in all previous versions of Deno that have AbortSignal.reason.

@lucacasonato lucacasonato changed the title Breaking change in deno v1.21.0 AbortSignal.reason not available in lib.dom.d.ts Apr 21, 2022
@jaydenseric
Copy link
Author

I'm still confused.

  1. The only change is the Deno version. If my types were wrong, why did it work with the last Deno version?
  2. Why does current version Deno type checking only notice a type error in a particular module when using deno test, but not deno check?

What can we do to overcome this?

As far as I can tell I'm not doing anything incorrectly in useOnClickRouteLink.mjs:

// @ts-check
/// <reference no-default-lib="true" />
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
/// <reference lib="dom.asynciterable" />
/// <reference lib="deno.ns" />
/// <reference lib="deno.unstable" />

Are you suggesting not writing type safe code that uses the DOM, so as to avoid the dom TS lib? The type error is raised in Deno std, and I can't directly control what code is written in there.

@lucacasonato
Copy link
Member

The only change is the Deno version

No your std version also changed. 0.136 was released on 3 hours ago.

@jaydenseric
Copy link
Author

  • deno@1.20.6 with std@0.134.0 works.
  • deno@1.20.6 with std@0.135.0 works.
  • deno@1.20.6 with std@0.136.0 works.
  • deno@1.21.0 with std@0.134.0 doesn't work.
  • deno@1.21.0 with std@0.135.0 doesn't work.
  • deno@1.21.0 with std@0.136.0 doesn't work.

The only thing that makes a difference between working or not is the Deno version.

@lucacasonato
Copy link
Member

Why does current version Deno type checking only notice a type error in a particular module when using deno test, but not deno check?

Because deno check's default type checking mode does not check remote modules (like deno run --no-check=remote)

@jaydenseric
Copy link
Author

Thanks, deno check --remote surfaces the error. That's a strange default. Why would it be more relaxed when you are explicitly running a command to check types, than when you are just running tests.

What can be done in project code to overcome this? I still can't tell what I've done wrong, if anything. Just downgrade Deno and wait for a future version with an updated TS dom lib that has AbortSignal.reason? Is that on the way sometime soon?

@jaydenseric
Copy link
Author

jaydenseric commented Apr 21, 2022

TS triple slash lib references are only meant to apply to the module with the comment in it and down in the module graph. As far as I can see, no module is referencing that dom lib above std in my module graph. So why is it affected? Is Deno v1.21.0 applying these triple slash lib references incorrectly to the module graph?

Edit: Maybe that's not true, when you factor in imported types in JSDoc type comments. Also, the React types from esm.sh might set the dom lib? Here is the module graph in question:

deno info --import-map=importMap.json useOnClickRouteLink.mjs
Output
local: [redacted]/ruck/useOnClickRouteLink.mjs
type: Mjs
dependencies: 74 unique (total 1.65MB)

file://[redacted]/ruck/useOnClickRouteLink.mjs (946B)
├─┬ file://[redacted]/ruck/useNavigate.mjs (343B)
│ ├─┬ file://[redacted]/ruck/NavigateContext.mjs (303B)
│ │ ├─┬ file://[redacted]/ruck/ClientProvider.mjs (8.52KB)
│ │ │ ├─┬ file://[redacted]/ruck/Effect.mjs (498B)
│ │ │ │ ├─┬ https://esm.sh/react@17.0.2?dev (196B)
│ │ │ │ │ ├─┬ https://cdn.esm.sh/v77/@types/react@17.0.44/index.d.ts (148.16KB)
│ │ │ │ │ │ ├── https://cdn.esm.sh/v77/@types/react@17.0.44/global.d.ts (7.21KB)
│ │ │ │ │ │ ├── https://cdn.esm.sh/v77/@types/prop-types@15.7.5/index.d.ts (3.58KB)
│ │ │ │ │ │ ├── https://cdn.esm.sh/v77/@types/scheduler@0.16.2/tracing.d.ts (4.03KB)
│ │ │ │ │ │ └── https://cdn.esm.sh/v77/csstype@3.0.11/index.d.ts (850.38KB)
│ │ │ │ │ ├── https://cdn.esm.sh/v77/react@17.0.2/deno/react.development.js (64.8KB)
│ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ ├─┬ file://[redacted]/ruck/HeadManager.mjs (3.1KB)
│ │ │ │ ├── https://esm.sh/react@17.0.2?dev *
│ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ ├─┬ file://[redacted]/ruck/HeadManagerContext.mjs (272B)
│ │ │ │ ├── file://[redacted]/ruck/HeadManager.mjs *
│ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ ├── file://[redacted]/ruck/NavigateContext.mjs *
│ │ │ ├─┬ file://[redacted]/ruck/RouteContext.mjs (256B)
│ │ │ │ ├─┬ file://[redacted]/ruck/serve.mjs (11.71KB)
│ │ │ │ │ ├── file://[redacted]/ruck/HeadManager.mjs *
│ │ │ │ │ ├── file://[redacted]/ruck/HeadManager.mjs *
│ │ │ │ │ ├── file://[redacted]/ruck/HeadManagerContext.mjs *
│ │ │ │ │ ├─┬ file://[redacted]/ruck/Html.mjs (555B)
│ │ │ │ │ │ ├── file://[redacted]/ruck/serve.mjs *
│ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├── file://[redacted]/ruck/RouteContext.mjs *
│ │ │ │ │ ├─┬ file://[redacted]/ruck/TransferContext.mjs (342B)
│ │ │ │ │ │ ├── file://[redacted]/ruck/serve.mjs *
│ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├── file://[redacted]/ruck/assertImportMap.mjs (3.14KB)
│ │ │ │ │ ├── file://[redacted]/ruck/assertImportMap.mjs *
│ │ │ │ │ ├─┬ file://[redacted]/ruck/publicFileResponse.mjs (2.42KB)
│ │ │ │ │ │ ├── file://[redacted]/ruck/serve.mjs *
│ │ │ │ │ │ ├─┬ https://deno.land/x/media_types@v3.0.2/mod.ts (4.64KB)
│ │ │ │ │ │ │ ├─┬ https://deno.land/std@0.131.0/path/mod.ts (800B)
│ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/_util/os.ts (598B)
│ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/_interface.ts (728B)
│ │ │ │ │ │ │ │ ├─┬ https://deno.land/std@0.131.0/path/common.ts (1.16KB)
│ │ │ │ │ │ │ │ │ └─┬ https://deno.land/std@0.131.0/path/separator.ts (259B)
│ │ │ │ │ │ │ │ │   └── https://deno.land/std@0.131.0/_util/os.ts *
│ │ │ │ │ │ │ │ ├─┬ https://deno.land/std@0.131.0/path/glob.ts (12.37KB)
│ │ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/_util/os.ts *
│ │ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/_util/os.ts *
│ │ │ │ │ │ │ │ │ ├─┬ https://deno.land/std@0.131.0/path/posix.ts (14.57KB)
│ │ │ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/_constants.ts (1.97KB)
│ │ │ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/_interface.ts *
│ │ │ │ │ │ │ │ │ │ └─┬ https://deno.land/std@0.131.0/path/_util.ts (3.69KB)
│ │ │ │ │ │ │ │ │ │   ├── https://deno.land/std@0.131.0/path/_constants.ts *
│ │ │ │ │ │ │ │ │ │   └── https://deno.land/std@0.131.0/path/_interface.ts *
│ │ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/separator.ts *
│ │ │ │ │ │ │ │ │ └─┬ https://deno.land/std@0.131.0/path/win32.ts (29.1KB)
│ │ │ │ │ │ │ │ │   ├── https://deno.land/std@0.131.0/_util/assert.ts (443B)
│ │ │ │ │ │ │ │ │   ├── https://deno.land/std@0.131.0/path/_constants.ts *
│ │ │ │ │ │ │ │ │   ├── https://deno.land/std@0.131.0/path/_interface.ts *
│ │ │ │ │ │ │ │ │   └── https://deno.land/std@0.131.0/path/_util.ts *
│ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/posix.ts *
│ │ │ │ │ │ │ │ ├── https://deno.land/std@0.131.0/path/separator.ts *
│ │ │ │ │ │ │ │ └── https://deno.land/std@0.131.0/path/win32.ts *
│ │ │ │ │ │ │ └── https://raw.githubusercontent.com/jshttp/mime-db/v1.52.0/db.json (181.53KB)
│ │ │ │ │ │ ├── https://deno.land/std@0.136.0/http/http_status.ts (6.5KB)
│ │ │ │ │ │ └─┬ https://deno.land/std@0.136.0/streams/conversion.ts (14.83KB)
│ │ │ │ │ │   └─┬ https://deno.land/std@0.136.0/io/buffer.ts (30.87KB)
│ │ │ │ │ │     ├── https://deno.land/std@0.136.0/_util/assert.ts (443B)
│ │ │ │ │ │     ├── https://deno.land/std@0.136.0/bytes/bytes_list.ts (3.99KB)
│ │ │ │ │ │     ├─┬ https://deno.land/std@0.136.0/bytes/mod.ts (7.23KB)
│ │ │ │ │ │     │ └── https://deno.land/std@0.136.0/bytes/equals.ts (1.46KB)
│ │ │ │ │ │     └── https://deno.land/std@0.136.0/io/types.d.ts (3.26KB)
│ │ │ │ │ ├─┬ file://[redacted]/ruck/readImportMapFile.mjs (1.24KB)
│ │ │ │ │ │ ├── file://[redacted]/ruck/assertImportMap.mjs *
│ │ │ │ │ │ └── file://[redacted]/ruck/assertImportMap.mjs *
│ │ │ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs (2.52KB)
│ │ │ │ │ ├─┬ https://unpkg.com/graphql-react@18.0.0/CacheContext.mjs (410B)
│ │ │ │ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├─┬ https://unpkg.com/graphql-react@18.0.0/Loading.mjs (1.7KB)
│ │ │ │ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ │ └─┬ https://unpkg.com/graphql-react@18.0.0/LoadingCacheValue.mjs (4.38KB)
│ │ │ │ │ │   ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ │   ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ │   ├── https://unpkg.com/graphql-react@18.0.0/Loading.mjs *
│ │ │ │ │ │   ├── https://unpkg.com/graphql-react@18.0.0/Loading.mjs *
│ │ │ │ │ │   └─┬ https://unpkg.com/graphql-react@18.0.0/cacheEntrySet.mjs (868B)
│ │ │ │ │ │     ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ │     └── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ │ │ ├─┬ https://unpkg.com/graphql-react@18.0.0/LoadingContext.mjs (428B)
│ │ │ │ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Loading.mjs *
│ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├─┬ https://esm.sh/react-dom@17.0.2/server?dev (217B)
│ │ │ │ │ │ ├─┬ https://cdn.esm.sh/v77/@types/react-dom@17.0.15/server~.d.ts (1.89KB)
│ │ │ │ │ │ │ └── https://cdn.esm.sh/v77/@types/react@17.0.44/index.d.ts *
│ │ │ │ │ │ ├─┬ https://cdn.esm.sh/v77/react-dom@17.0.2/deno/server.development.js (135.4KB)
│ │ │ │ │ │ │ └── https://cdn.esm.sh/v77/react@17.0.2/deno/react.development.js *
│ │ │ │ │ ├─┬ https://unpkg.com/react-waterfall-render@4.0.0/waterfallRender.mjs (4.71KB)
│ │ │ │ │ │ ├─┬ https://unpkg.com/react-waterfall-render@4.0.0/WaterfallRenderContext.mjs (1.01KB)
│ │ │ │ │ │ │ ├── https://unpkg.com/react-waterfall-render@4.0.0/waterfallRender.mjs *
│ │ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ │ │ ├── https://deno.land/std@0.136.0/http/http_status.ts *
│ │ │ │ │ ├─┬ https://deno.land/std@0.136.0/http/server.ts (20.29KB)
│ │ │ │ │ │ └─┬ https://deno.land/std@0.136.0/async/mod.ts (322B)
│ │ │ │ │ │   ├─┬ https://deno.land/std@0.136.0/async/abortable.ts (2KB)
│ │ │ │ │ │   │ └── https://deno.land/std@0.136.0/async/deferred.ts (1.49KB)
│ │ │ │ │ │   ├─┬ https://deno.land/std@0.136.0/async/deadline.ts (618B)
│ │ │ │ │ │   │ └── https://deno.land/std@0.136.0/async/deferred.ts *
│ │ │ │ │ │   ├── https://deno.land/std@0.136.0/async/debounce.ts (2.08KB)
│ │ │ │ │ │   ├── https://deno.land/std@0.136.0/async/deferred.ts *
│ │ │ │ │ │   ├── https://deno.land/std@0.136.0/async/delay.ts (827B)
│ │ │ │ │ │   ├─┬ https://deno.land/std@0.136.0/async/mux_async_iterator.ts (1.97KB)
│ │ │ │ │ │   │ └── https://deno.land/std@0.136.0/async/deferred.ts *
│ │ │ │ │ │   ├── https://deno.land/std@0.136.0/async/pool.ts (2.58KB)
│ │ │ │ │ │   └── https://deno.land/std@0.136.0/async/tee.ts (2.21KB)
│ │ │ │ │ └─┬ https://deno.land/std@0.136.0/path/mod.ts (800B)
│ │ │ │ │   ├── https://deno.land/std@0.136.0/_util/os.ts (598B)
│ │ │ │ │   ├── https://deno.land/std@0.136.0/path/_interface.ts (728B)
│ │ │ │ │   ├─┬ https://deno.land/std@0.136.0/path/common.ts (1.16KB)
│ │ │ │ │   │ └─┬ https://deno.land/std@0.136.0/path/separator.ts (259B)
│ │ │ │ │   │   └── https://deno.land/std@0.136.0/_util/os.ts *
│ │ │ │ │   ├─┬ https://deno.land/std@0.136.0/path/glob.ts (12.37KB)
│ │ │ │ │   │ ├── https://deno.land/std@0.136.0/_util/os.ts *
│ │ │ │ │   │ ├── https://deno.land/std@0.136.0/_util/os.ts *
│ │ │ │ │   │ ├─┬ https://deno.land/std@0.136.0/path/posix.ts (14.64KB)
│ │ │ │ │   │ │ ├── https://deno.land/std@0.136.0/path/_constants.ts (1.97KB)
│ │ │ │ │   │ │ ├── https://deno.land/std@0.136.0/path/_interface.ts *
│ │ │ │ │   │ │ └─┬ https://deno.land/std@0.136.0/path/_util.ts (3.69KB)
│ │ │ │ │   │ │   ├── https://deno.land/std@0.136.0/path/_constants.ts *
│ │ │ │ │   │ │   └── https://deno.land/std@0.136.0/path/_interface.ts *
│ │ │ │ │   │ ├── https://deno.land/std@0.136.0/path/separator.ts *
│ │ │ │ │   │ └─┬ https://deno.land/std@0.136.0/path/win32.ts (29.18KB)
│ │ │ │ │   │   ├── https://deno.land/std@0.136.0/_util/assert.ts *
│ │ │ │ │   │   ├── https://deno.land/std@0.136.0/path/_constants.ts *
│ │ │ │ │   │   ├── https://deno.land/std@0.136.0/path/_interface.ts *
│ │ │ │ │   │   └── https://deno.land/std@0.136.0/path/_util.ts *
│ │ │ │ │   ├── https://deno.land/std@0.136.0/path/posix.ts *
│ │ │ │ │   ├── https://deno.land/std@0.136.0/path/separator.ts *
│ │ │ │ │   └── https://deno.land/std@0.136.0/path/win32.ts *
│ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ ├── file://[redacted]/ruck/scrollToHash.mjs (699B)
│ │ │ ├── file://[redacted]/ruck/serve.mjs *
│ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Cache.mjs *
│ │ │ ├── https://unpkg.com/graphql-react@18.0.0/CacheContext.mjs *
│ │ │ ├─┬ https://unpkg.com/graphql-react@18.0.0/HydrationTimeStampContext.mjs (471B)
│ │ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ │ ├── https://unpkg.com/graphql-react@18.0.0/Loading.mjs *
│ │ │ ├── https://unpkg.com/graphql-react@18.0.0/LoadingContext.mjs *
│ │ │ ├── https://esm.sh/react@17.0.2?dev *
│ │ │ └── https://esm.sh/react@17.0.2?dev *
│ │ └── https://esm.sh/react@17.0.2?dev *
│ └── https://esm.sh/react@17.0.2?dev *
├── https://esm.sh/react@17.0.2?dev *
└── https://esm.sh/react@17.0.2?dev *

@MierenManz
Copy link

MierenManz commented Apr 21, 2022

Why would it be more relaxed when you are explicitly running a command to check types, than when you are just running tests.

This is due to the new change that is gonna happen in Deno 2.0 (most likely). Typechecking is super slow especially once you get alot of bigger deps. Therefore it will be the modules responsibility to typecheck their code (which they already do because of unit tests, benchmarks and testing their code manually) and your responsibility to typecheck your's rather than you typechecking everything.
This should speed up start up time without having any real drawback

@bartlomieju
Copy link
Member

@jaydenseric the explanation for deno check just landed in the blog post https://deno.com/blog/v1.21#deno-check-and-the-path-to-not-type-checking-by-default

@dsherret
Copy link
Member

dsherret commented Apr 21, 2022

TS triple slash lib references are only meant to apply to the module with the comment in it and down in the module graph.

No, they're global. There's no way to do that in TS at the moment :(

@jaydenseric
Copy link
Author

jaydenseric commented Apr 22, 2022

Thanks for the responses. I'm losing confidence there is a workable way to have type-safety in dual Deno/browser codebases, which is one of main practical uses of Deno 😩

It now seems like an anti-pattern to use /// <reference no-default-lib="true" /> in published modules, because that will obliterate whatever TS libs the consumer had configured for their project modules? It would be a race condition too, as whatever module is processed first (or last?) would override others? I'd been using it when using these comments at the top of modules that use DOM APIs:

// @ts-check
/// <reference no-default-lib="true" />
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
/// <reference lib="dom.asynciterable" />
/// <reference lib="deno.ns" />
/// <reference lib="deno.unstable" />

For example:

https://github.com/jaydenseric/ruck/blob/5533d25f9f4bd244618e6bc3683e6690f00c7b9d/scrollToHash.mjs#L2

The dom lib needs to be referenced in individual modules because you can't predict which ones will be deep imported.

Just the act of loading the dom lib currently prohibits the use of any Deno std modules that may pull something in using AbortSignal.reason, which is perfectly safe to do in Deno if not for the types exploding. It could happen for other DOM APIs too, which is scary as I can't see what you can do to defend against it or fix it in published modules or your projects. TypeScript team take a long time to update lib definitions, and then you have to wait another length of time for Deno to update the TypeScript version and libs following that.

For example, TypeScript accounted for AbortSignal.reason in the dom lib far upstream on March 20 (a month ago):

https://github.com/microsoft/TypeScript-DOM-lib-generator/pull/1294/files#diff-d57c51d024219b9f37e6b42df127db25f700acbaade23ce1edcecda2853b995cR1964

But that won't make it into TypeScript until v4.7, which isn't planned for release until May 24th (a month away):

https://github.com/microsoft/TypeScript/blob/6e62273fa1e7469b89b589667c2c233789c62176/lib/lib.dom.d.ts#L1984

Presumably it won't be available in Deno until a little bit after then:

#14242

Perhaps we need a totally different approach to using DOM globals in a type safe way. Ideally there would be a way to use typed DOM APIs within just part of a Deno module, for example within a React useEffect hook callback, or within Puppeteer page.evaluate scope in Deno test modules.

Maybe make a library of DOM API types that can be imported inline, on demand:

// @ts-check

const doc = /** @type {import("dom-types/document.mjs")} */ (
  // @ts-ignore This really does exist.
  document
);
cost foo = doc.getElementById("bar");

This is assuming TypeScript itself couldn't be enhanced to have a way to declare the type of a particular thing in a particular scope. Perhaps a JSDoc @typedef could be allowed to conflict with an existing symbol as a way to override it everywhere in the current scope?

// @ts-check

// This module scope sometimes runs in a non-DOM Deno environment.

import { useEffect } from "react";

export default function Foo() {
  useEffect(() => {
    // This function scope only ever runs in a DOM environment.

    /** @typedef {import("dom-types/document.mjs")} document **/

    cost a = document.getElementById("b");
    cost c = document.getElementById("d");
  }, []);

  return null;
}

We need a documented right way to do universal Deno/browser code ASAP; we can't really have the confidence to practically use Deno or publish Deno modules until there is.

@aapoalas
Copy link
Collaborator

aapoalas commented Apr 22, 2022

Given the example of code such as this:

export const foo = () => {
  return document.createElement("div");
};

generic TS code with DOM typings is correct in saying that this function is fine and all types are correct. Deno is also correct in saying that the body of the function contains an error and the types reflect that error properly.

TS wise the existing ways to work around this issue would be either @ts-ignore or @ts-expect-error. The former would turn off all type errors from that line for both Deno and browser facing TS code, so it's not a particularly appealing solution. The latter would error on browser facing TS code, so doesn't really work.

I wonder if Deno could implement its own @deno-expect-error directive that could be used to suppress these sorts of errors?

Another thing I wonder about is what happens if instead of triple-slash referencing you actually properly import the type declaration file? Would that then keep the type declarations inside the module subtree?

EDIT: Direct import does not keep the types inside the module subtree :(

@pcj
Copy link

pcj commented Apr 24, 2022

I'm also hitting this and don't know what to do. I'm a relative newcomer to deno, so maybe it's just me. My deps blow up when I uncomment gfm:

import { Component } from "https://deno.land/x/nano_jsx@v0.0.30/mod.ts";

export type ReactNode = Component | Component[] | string | string[] | number | number[]

export * from "https://deno.land/x/nano_jsx@v0.0.30/mod.ts";
export { tw } from "https://cdn.skypack.dev/twind";
export { default as Prism } from "https://esm.sh/prismjs@1.28.0";
export { escape as htmlEscape } from "https://esm.sh/he@1.2.0";
export { emojify } from "https://deno.land/x/emoji@0.1.2/mod.ts";
export { default as twas } from "https://esm.sh/twas@2.1.2";
// export { CSS, render as gfm} from "https://deno.land/x/gfm@0.1.20/mod.ts";
error: TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
    return Promise.reject(createAbortError(signal.reason));
                                                  ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:27:51

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
  const abort = () => waiter.reject(createAbortError(signal.reason));
                                                            ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:30:61

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
    throw createAbortError(signal.reason);
                                  ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:46:35

TS2339 [ERROR]: Property 'reason' does not exist on type 'AbortSignal'.
  const abort = () => waiter.reject(createAbortError(signal.reason));
                                                            ~~~~~~
    at https://deno.land/std@0.136.0/async/abortable.ts:49:61

Found 4 errors.

Is there a workaround other than not using the gfm dependency?

% deno --version                       
deno 1.20.6 (release, aarch64-apple-darwin)
v8 10.0.139.6
typescript 4.6.2

@pcj
Copy link

pcj commented Apr 24, 2022

I ended up working around this by adding the following to my deps.ts. Still don't understand why it's necessary though...

declare global {
    interface AbortSignal {
        reason: string;
    }
}

@marbemac
Copy link

Just trying Deno out for the first time, and ran into this.

It seems that --compat doesn't work at all if dom is listed in the libs. This does not seem to be new to 1.21, I tried several 1.20.x versions and ran into the same issue.

deno.json:

{
  "compilerOptions": {
    "lib": [
      "dom",
      "dom.iterable",
      "dom.asynciterable",
      "deno.ns",
      "deno.unstable"
    ]
  }
}

hello.ts:

console.log("hello world");

Simply turning on compat triggers the AbortSignal.reason typing error - deno run --compat --unstable --allow-read hello.ts.

For now I've gotten around it by passing in --no-check=remote.

@kitsonk kitsonk self-assigned this Apr 29, 2022
kitsonk added a commit to kitsonk/deno that referenced this issue Apr 29, 2022
kitsonk added a commit that referenced this issue Apr 29, 2022
cjihrig pushed a commit that referenced this issue May 5, 2022
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 a pull request may close this issue.

9 participants