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

lib.dom.d.ts incompatible with built-in Deno libs #8070

Closed
kitsonk opened this issue Oct 22, 2020 · 9 comments
Closed

lib.dom.d.ts incompatible with built-in Deno libs #8070

kitsonk opened this issue Oct 22, 2020 · 9 comments
Labels
cli related to cli/ dir needs info needs further information to be properly triaged

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Oct 22, 2020

Any updates?

I got a lot of ts errors when I used @deno-types to include type definitions for the ky libarry.

// @deno-types="https://deno.land/x/ky@v0.23.0/index.d.ts"
export { default as ky } from "https://deno.land/x/ky@v0.23.0/index.js

From the error message, it is the lib.dom.d.ts which causes conflicts with other definitions.

TS2300 [ERROR]: Duplicate identifier 'Event'.
declare class Event {
              ~~~~~
    at asset:///lib.deno.web.d.ts:21:15

    'Event' was also declared here.
    interface Event {
              ~~~~~
        at asset:///lib.dom.d.ts:5285:11    and here.
    declare var Event: {
                ~~~~~
        at asset:///lib.dom.d.ts:5353:13
...

Originally posted by @cj1128 in #3726 (comment)

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 22, 2020

This is a regression, likely #7514 (cc/ @lucacasonato). We are not specifically testing for it but we should.

I have broken it out as a new issues, since the original issue and solution are actually unrelated.

Because they are classes now, instead of interfaces, they can't be merged/redeclared. There was something niggling me at the back of the mind about this, and it appears that this is the problem.

@kitsonk kitsonk added bug Something isn't working correctly cli related to cli/ dir labels Oct 22, 2020
@nayeemrmn
Copy link
Collaborator

For reference, you can get a list of incompatibilities by compiling anything with the following tsconfig:

{
  "compilerOptions": {
    "lib": [
      "dom",
      "deno.web",
      "deno.fetch",
      "deno.window",
      "deno.shared_globals",
      "deno.ns",
      "deno.unstable"
    ]
  }
}

I remember seeing a bunch of errors before #7514, enough that I didn't think it was a goal to have this. If it is, we can easily add an integration test.

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Oct 22, 2020

Although, I think we should just make "dom" mutually exclusive to Deno's web API libs. Having alignment prevents us from, say, partially populating the navigator object since we are forced to match its type in "dom".

It seems the quoted user can fix the issue by using the following tsconfig:

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

If not, we should instead focus on making this work.

EDIT: I guess it would be a --dom flag to use this config, one tsconfigs aren't settable.

@lucacasonato
Copy link
Member

remember seeing a bunch of errors before

Yeah me too. I don't think we should explicitly be supporting this.

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 22, 2020

Yeah me too. I don't think we should explicitly be supporting this.

I disagree... it is one of the main use cases for writing code that works on both the web and Deno. At a minimum we should support the following without error:

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

I may have mis-read the comment from the OP upon reflection... full compatibility is impossible with all the libs. It is just those 3 that need to not step on each other...

@lucacasonato
Copy link
Member

lucacasonato commented Oct 22, 2020

That already works. I use it in dext.

@kitsonk kitsonk added needs info needs further information to be properly triaged and removed bug Something isn't working correctly labels Oct 22, 2020
@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 22, 2020

Ah, ok, then we need clarification from the OP then. It looks like ky might need to change to be more Deno friendly.

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 28, 2020

Having the lib dom along with deno.ns was added in #8157 as well as it includes a test case of how it is intended to work in Deno: https://github.com/denoland/deno/pull/8157/files#diff-cdf6d22aefaa516c0cec95fed92e71764b0c00a4ccfc3c5f1f42b2429e50f98b (or a --config tsconfig.json can be used).

@kitsonk kitsonk closed this as completed Oct 28, 2020
@cj1128
Copy link

cj1128 commented Oct 28, 2020

Yeah me too. I don't think we should explicitly be supporting this.

I disagree... it is one of the main use cases for writing code that works on both the web and Deno. At a minimum we should support the following without error:

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

I may have mis-read the comment from the OP upon reflection... full compatibility is impossible with all the libs. It is just those 3 that need to not step on each other...

I tried and this worked. Thank you!

applmak added a commit to google/chicago-brick that referenced this issue Nov 30, 2022
- Don't use <reference lib="dom". Instead, use deno.json. See denoland/deno#8070 for details.
- Upgrade to the latest std lib.
- Fix some unqualified module imports.
applmak added a commit to google/chicago-brick that referenced this issue Nov 30, 2022
- Don't use <reference lib="dom". Instead, use deno.json. See denoland/deno#8070 for details.
- Upgrade to the latest std lib.
- Fix some unqualified module imports.
applmak added a commit to google/chicago-brick that referenced this issue Nov 30, 2022
- Don't use <reference lib="dom". Instead, use deno.json. See denoland/deno#8070 for details.
- Upgrade to the latest std lib.
- Fix some unqualified module imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir needs info needs further information to be properly triaged
Projects
None yet
Development

No branches or pull requests

4 participants