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

RxJS 6 requires "dom" lib when using TypeScript #3558

Closed
imhoffd opened this issue Apr 12, 2018 · 4 comments · Fixed by #3566
Closed

RxJS 6 requires "dom" lib when using TypeScript #3558

imhoffd opened this issue Apr 12, 2018 · 4 comments · Fixed by #3566
Assignees

Comments

@imhoffd
Copy link

imhoffd commented Apr 12, 2018

RxJS version: 6.0.0-beta.4

Code to reproduce:

index.ts

import { Observable } from 'rxjs';

tsconfig.json

{
  "compilerOptions": {
    "target": "es6",
    "module": "commonjs",
    "moduleResolution": "node",
    "strict": true,
    "lib": [
      "es2015"
    ]
  },
  "files": [
    "index.ts"
  ]
}

Expected behavior: Successful compilation.

Actual behavior: Without "dom" in "lib", the following compilation error occurs:

❯ npx tsc --project tsconfig.json
node_modules/rxjs/internal/observable/fromEvent.d.ts(10,39): error TS2304: Cannot find name 'EventTarget'.
node_modules/rxjs/internal/observable/fromEvent.d.ts(10,103): error TS2304: Cannot find name 'NodeList'.
node_modules/rxjs/internal/observable/fromEvent.d.ts(10,114): error TS2304: Cannot find name 'HTMLCollection'.
@cartant
Copy link
Collaborator

cartant commented Apr 13, 2018

This is down to having all of the factory functions - including fromEvent - now exported from within "rxjs/index.d.ts".

The distribution now relies upon a bundler to 'tree-shake' any unused exports. However, TypeScript will still see all of the exports - whether used or not. That means that when using RxJS v6 in a Node application, it's still necessary to include TypeScript's dom library.

This has the unfortunate effect of introducing a whole bunch of type and global declarations that are not appropriate for a Node target.

In v5, the dom TypeScipt lib would only have been required if fromEvent was used - because fromEvent supports Node and DOM event sources.

Maybe there's not too much that can be done about this - short of removing the dependency on the EventTarget, NodeList and HTMLCollection type declarations?

@imhoffd
Copy link
Author

imhoffd commented Apr 13, 2018

You could ship two sets of .d.ts files: browser and Node. This would offer a workaround for Node users using TypeScript's paths compiler option:

{
  "compilerOptions": {
    "baseUrl": ".",
    "target": "es6",
    "module": "commonjs",
    "moduleResolution": "node",
    "strict": true,
    "paths": {
      "rxjs": ["node_modules/rxjs/node/index.d.ts"]
    },
    "lib": [
      "es2015"
    ]
  },
  "files": [
    "index.ts"
  ]
}

@jasonaden
Copy link
Collaborator

You can also add the option skipLibCheck. Without this, you're re-typechecking d.ts files that were compiled during the rxjs build process. You don't need to re-typecheck these since they were type checked when produced.

This could be even more obvious if, for instance, a library you depend on had looser compilation flags than your project. Say the library had noImplicitAny: false and you set it to true. You wouldn't want a bunch of compilation errors since there's really nothing you can do about it.

Therefore you should typically be using skipLibCheck to ignore this type of warning.

@benlesh benlesh self-assigned this Apr 13, 2018
benlesh added a commit to benlesh/rxjs that referenced this issue Apr 13, 2018
Adds interfaces to cover what is used from DOM in `fromEvent` so that types are carried through, but `dom` lib isn not required by default even if you are not using `fromEvent`

fixes: ReactiveX#3558
benlesh added a commit that referenced this issue Apr 13, 2018
Adds interfaces to cover what is used from DOM in `fromEvent` so that types are carried through, but `dom` lib isn not required by default even if you are not using `fromEvent`

fixes: #3558
@lock
Copy link

lock bot commented Jun 5, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants