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

Could not find a declaration file for module '@reactivex/rxjs' (5.5.x) #3005

Closed
mdekrey opened this issue Oct 26, 2017 · 7 comments · Fixed by #3027
Closed

Could not find a declaration file for module '@reactivex/rxjs' (5.5.x) #3005

mdekrey opened this issue Oct 26, 2017 · 7 comments · Fixed by #3027

Comments

@mdekrey
Copy link

mdekrey commented Oct 26, 2017

RxJS version:
5.5.0, 5.5.1, 5.5.2

Code to reproduce:
index.ts

import { Subject } from "@reactivex/rxjs";

const test = new Subject<boolean>();

test.next(true);

package.json

{
  "dependencies": {
    "@reactivex/rxjs": "5.5.0",
    "typescript": "^2.5.3"
  }
}

*tsconfig.json*
```json
{
    "compilerOptions": {
        "noImplicitAny": true
    }
}

Expected behavior:
No errors.

Actual behavior:

index.ts(1,25): error TS7016: Could not find a declaration file for module '@reactivex/rxjs'. 'C:/Users/mattd/Source/Examples/rxjs-bug/node_modules/@reactivex/rxjs/index.js' implicitly has an 'any' type.
Try npm install @types/@reactivex/rxjs if it exists or add a new declaration (.d.ts) file containing declare module '@reactivex/rxjs';

Additional information:
The package.json of rxjs contains "typings": "./dist/package/typings/Rx.d.ts", but there is no typings folder in the package directory. I expect that it should be changed to "typings": "./dist/typings/Rx.d.ts", because that appears to work.

@davideast
Copy link

Thanks for taking the time to file the issue @mdekrey! I confirmed this myself. The package.json needs to point to "typings": "./dist/typings/Rx.d.ts", or typings needs to be in ./dist/package.

Thoughts @benlesh?

@kwonoj
Copy link
Member

kwonoj commented Oct 30, 2017

/cc @jasonaden as well for visibility.

jasonaden added a commit to jasonaden/rxjs that referenced this issue Oct 30, 2017
@jasonaden
Copy link
Collaborator

@kwonoj I don't know if we need two separate PRs for this as they are the same fix. But I sent both just to make sure we don't miss the merge on either stable or master branch.

@kwonoj
Copy link
Member

kwonoj commented Oct 30, 2017

@jasonaden I think for these kind PR we may need who merges pr manually cherrypick into master to avoid ppl have to create same pr twice, but for now for opened PR I also think it should be fine.

@jasonaden
Copy link
Collaborator

@kwonoj Agreed. On the Angular project we adopted certain labels so the person merging knows what to do. And we recently wrote scripts that will do rebasing based on the labels. Might want to adopt something similar. It's basically covered in this doc and has drastically simplified the process of merging PRs to the correct branches.

@mdekrey
Copy link
Author

mdekrey commented Nov 1, 2017

Thanks for taking care of this @kwonoj and @jasonaden! Do you know when we'll get a new release from stable?

@lock
Copy link

lock bot commented Jun 6, 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 6, 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