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

[v7] TS automatic imports in VSCode default to wrong path #6067

Closed
voliva opened this issue Mar 1, 2021 · 12 comments · Fixed by #6276
Closed

[v7] TS automatic imports in VSCode default to wrong path #6067

voliva opened this issue Mar 1, 2021 · 12 comments · Fixed by #6276
Assignees
Labels
bug Confirmed bug

Comments

@voliva
Copy link
Contributor

voliva commented Mar 1, 2021

Bug Report

Current Behavior
When using operators in VSCode, the suggested import defaults to rxjs/dist/types/operators, failing in runtime.

This wasn't happening in v6, starts happening after upgrading to v7.

Expected behavior
VSCode imports from the correct rxjs/operators path

Reproduction

  • create a simple playground
mkdir rxjs-playground
cd rxjs-playground
npm init -y
npm i rxjs@next
  • open that folder in VSCode
  • create a file foo.ts
interval(1000).pipe( // Correctly suggests `import { interval } from 'rxjs'`
   map(v => v*2) // Doesn't suggest any valid import, that's fine - it used to do that. Manually import it from `rxjs/operators`
)
  • create a file bar.ts and type in:
interval(1000).pipe( // Correctly suggests `import { interval } from 'rxjs'`
   map(v => v*2) // Wrongly suggests `import { map } from 'rxjs/dist/types/operators'`
)

Environment

  • RxJS version: 7.0.0-beta.12
  • VSCode: 1.53.2
  • Node: 12.18.3
  • Typescript: 4.1.5

Additional context/Screenshots

image

Note that after correcting it to rxjs/operators it correctly suggests adding the import from the right place

image

@benlesh
Copy link
Member

benlesh commented Mar 1, 2021

This issue in TypeScript seems related: microsoft/TypeScript#30472

I'm not really sure what the resolution to this is. If it's on our end, or if it's in your tsconfig.

@cartant @RyanCavanaugh any ideas?

@benlesh
Copy link
Member

benlesh commented Mar 1, 2021

@voliva do you have to same issues with RxJS 6.x?

@benlesh
Copy link
Member

benlesh commented Mar 1, 2021

One primary difference I see between 6.x and 7.x is in 6.x we have the CJS root and typings root right next to each other. Where they are not next to each other in 7.x:

6.x package.json:

{
"main": "./index.js",
"typings": "./index.d.ts",
"module": "./_esm5/index.js",
"es2015": "./_esm2015/index.js"
}

7.x package.json:

{
"main": "./dist/cjs/index.js",
"types": "./dist/types/index.d.ts",
"module": "./dist/esm5/index.js",
"es2015": "./dist/esm/index.js",
}

@voliva
Copy link
Contributor Author

voliva commented Mar 1, 2021

@voliva do you have to same issues with RxJS 6.x?

No, with RxJS 6.x it seems to be working fine - It happened on both projects of mine that I upgraded to try 7.x, both of them on two different computers, one running MacOS Catalina, the other Windows 10. And the minimal example I posted in the issue description also causes that.

I'm sorry I can't help much - I've tried some combinations that would make sense with the new dist structure, but it seems I can't get it to work.

My colleague uses neovim and it seems that he doesn't have this issue on his editor. Maybe I should take this opportunity to give vim another try 😅. Is it reproducible by other VSCode users though?

@benlesh benlesh added this to High priority in Core Issues Triage Mar 1, 2021
@cartant
Copy link
Collaborator

cartant commented Mar 1, 2021

any ideas?

TBH, I don't think there's a great deal that can be done until the exports property in package.json becomes widely supported and we start using it. ATM, it's not reasonable, IMO, to expect tooling to determine that we have a valid import locations like rxjs/operators, rxjs/testing, etc., but that the directories under rxjs/dist are invalid.

Long-term, I think the deprecated names that conflict should be removed and everything should be imported from rxjs.

@cartant
Copy link
Collaborator

cartant commented Mar 1, 2021

any ideas?

I suppose we could also try renaming dist to a name that sorts last alphabetically and just hope that tools stumble across oprerators, etc., first.

@benlesh
Copy link
Member

benlesh commented Apr 15, 2021

I suppose we could also try renaming dist to a name that sorts last alphabetically and just hope that tools stumble across oprerators, etc., first.

Haha... zzzzzzzzzzzzzzzzzzzdist.

@benlesh
Copy link
Member

benlesh commented Apr 15, 2021

FWIW: We do now have the exports set in our package.json.

benlesh added a commit to benlesh/rxjs that referenced this issue Apr 16, 2021
This is an attempt to fix issues with VS Code auto-import, as described here in this issue: microsoft/TypeScript#43034

Related ReactiveX#6067
benlesh added a commit that referenced this issue Apr 19, 2021
This is an attempt to fix issues with VS Code auto-import, as described here in this issue: microsoft/TypeScript#43034

Related #6067
@benlesh
Copy link
Member

benlesh commented Apr 19, 2021

This is now resolved in the latest 7.0.0-rc.1. From #6229

@benlesh benlesh closed this as completed Apr 19, 2021
Core Issues Triage automation moved this from High priority to Closed Apr 19, 2021
@BlackGlory
Copy link

BlackGlory commented Apr 20, 2021

In the rc.1 version, I get "error TS2307: Cannot find module 'rxjs' or its corresponding type declarations.", is this a breaking change?
I am using TypeScript 4.2.4 + VSCode 1.55.2.

@benlesh benlesh reopened this Apr 20, 2021
Core Issues Triage automation moved this from Closed to Needs triage Apr 20, 2021
@benlesh
Copy link
Member

benlesh commented Apr 20, 2021

While the applied fix did fix the issue in VS Code, it also broke tsc's ability to find the rxjs module at all. I have no idea how that is possible, but I'm reverting the fix and cutting rc2

@benlesh benlesh added the bug Confirmed bug label Apr 28, 2021
@benlesh benlesh self-assigned this Apr 28, 2021
benlesh added a commit to benlesh/rxjs that referenced this issue Apr 28, 2021
- Adds typesVersions, enforcing TypeScript >= 4.2.
- Adds some superfluous imports required for TS and VS Code to find proper import locations for our strange deep import sites.
- Independantly tested by building the package and installing it locally in another project, then testing in VS code.

Fixes ReactiveX#6067
Related microsoft/TypeScript#43034
benlesh added a commit to benlesh/rxjs that referenced this issue Apr 28, 2021
- Adds typesVersions, enforcing TypeScript >= 4.2.
- Adds some superfluous references required for TS and VS Code to find proper import locations for our strange deep import sites.
- Independantly tested by building the package and installing it locally in another project, then testing in VS code.

Fixes ReactiveX#6067
Related microsoft/TypeScript#43034
Core Issues Triage automation moved this from Needs triage to Closed Apr 29, 2021
benlesh added a commit that referenced this issue Apr 29, 2021
- Adds typesVersions, enforcing TypeScript >= 4.2.
- Adds some superfluous references required for TS and VS Code to find proper import locations for our strange deep import sites.
- Independantly tested by building the package and installing it locally in another project, then testing in VS code.

Fixes #6067
Related microsoft/TypeScript#43034
@lamisChebbi
Copy link

Hi

I have the same issue when importing Subjects with rxjs@7.0.1. The wrong import results in a runtime error. This works fine with rxjs 6.
As illustrated in the screenshot below I'm using the same version of VSCode for two Angular projects.
The rxjs 6 version works fine while the rxjs7 doesn't (so most likely it is not a VScode issue)

TypeScript: 4.2.4
image

Do you have any idea about this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants