Auto imports not working properly with RxJS 7 beta

[benlesh]

• VS Code Version: any
• OS Version: any

Steps to Reproduce:

1. Add rxjs@7.0.0-beta.12 to a project
2. type concatMap and check the possible auto-import locations
3. It should show rxjs/operators as the correct import location. Instead it shows rxjs/dist/types/operators.

Does this issue occur when all extensions are disabled?: Yes

This comes from an issue reported on the RxJS repository here: ReactiveX/rxjs#6067

Apparently, neovim users don't experience this issue, so it leads me to believe it's not the configuration of the package.

[mjbvz]

Having trouble getting any auto imports for map to show up:

import { interval } from "rxjs";

interval(100).pipe(
    map()
)

There are no auto imports:

Any extra steps required while setting up the project?

[voliva]

Yes, first you need to import something from rxjs/operators in another file of the same project. This usually hints that rxjs/operators also has exports.

The issue in the other files (where it hasn't been manually imported), then VSCode suggests rxjs/dist/types/operators instead.

My environment, btw:

• VSCode: 1.53.2
• Node: 12.18.3
• Typescript: 4.1.5

[Toliak]

Reproduced for me with TS 4.2.3.

Yes, There is no suggestions to import concatMap (another bug, I think it is #30033), until there is no .ts files with rxjs/operators import.

After adding mentioned .ts file, for example:

import {} from 'rxjs/operators'

The bug appears:

OS Version: Linux manjaro 5.4.97-1-MANJARO

Code Version:

1.53.1
5d424b828ada08e1eb9f95d6cb41120234ef57c7
x64

All extensions are disabled.

[voliva]

I've been digging a bit around TSServer and am I correct the fix might be in moduleSpecifiers.ts#tryGetModuleNameAsNodeModule?

As far as I can see, that function receives a declaration file path (e.g. {absolutePath}/node_modules/rxjs/dist/types/index.d.ts or {absolutePath}/node_modules/rxjs/dist/types/operators/index.d.ts) and tries to resolve it to a node module name by looking at package.json, correctly identifying rxjs/dist/types/index.d.ts as rxjs but failing to identifiy rxjs/dist/types/operators/index.d.ts

Would it be possible to update this file to also consider subfolders?

[andrewbranch]

This is caused by the highly unusual package organization present in RxJS 7. node_modules/rxjs/operators is empty except for a package.json (not the root package.json) whose entrypoints point back outside of that folder (to dist/types/operators/index.d.ts for types). While that works fine for module resolution, it’s hard to imagine how we should be able to come up with that from scratch. When we generate a module specifier for a module file inside node_modules, we walk up the directory tree from that file looking at package.json files we see along the way, and see if their types/main fields point to that file. If so, we can use a short path to the directory of that package.json. But when we start at node_modules/rxjs/dist/types/operators/index.d.ts, we of course never see node_modules/rxjs/operators/package.json in that traversal. From the perspective of the module specifier resolver, we have absolutely no indication that some other directory would contain a match for that file. I’m not sure there’s anything TypeScript can reasonably do here.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[andrewbranch]

@benlesh let me know if you can’t work out a way to get a good UX given this information and we can brainstorm solutions. Don’t want the closing to come off as dismissive; it just doesn’t seem like this is a bug on our end right now.

[voliva]

But when we start at node_modules/rxjs/dist/types/operators/index.d.ts, we of course never see node_modules/rxjs/operators/package.json in that traversal.

I wonder if it can be inferred in some way... The root's RxJS@7 package.json has

  "types": "./dist/types/index.d.ts",

When we use Observable and try to get import suggestions, Observable is exported from within ./dist/types/index.d.ts and we correctly receive to import from 'rxjs'

Now, if we use map and try to get import suggestions... why is TS Server looking at dist/types/operators/index.d.ts at the first place? Isn't there a way where we can use this information together with ./dist/types/index.d.ts to see that the relative path to dist/types/operators/index.d.ts is ./operators?

In a way, this map feels like it should be simple:

// Given package.json -> types = ./dist/types/index.d.ts

./dist/types/index.d.ts => 'rxjs'
./dist/types/operators/index.d.ts => 'rxjs/operators'

// extended to any structure
./dist/types/whatever/something/else/index.d.ts => 'rxjs/whatever/something/else'

Although I understand that it's not as straightforward.

Otherwise is there any suggestion on how to change the structure to support these kind of imports?

[andrewbranch]

In a way, this map feels like it should be simple

There’s no guarantee it had to work out that way. The relative paths match up because it made sense to a human to do it that way, but I could fork RxJS and make a version that breaks that expectation.

Otherwise is there any suggestion on how to change the structure to support these kind of imports?

There are lots of solutions, because you really have to be doing non-standard stuff to break this in the first place. In general, shipping types in a different subfolder (e.g. dist/types) from the rest of the JS is a recipe for pain. By far the happiest path is just shipping the JS and .d.ts that tsc gives you alongside each other. I’m sure there are lots of compelling Technical Reasons why RxJS is doing something different, but the farther you get from that happy path, the more you’re likely to subtly break editor features or have to pile on cascading workarounds to avoid breaking them.

[benlesh]

@andrewbranch rxjs now has the exports set in our package.json. Will that be helpful in resolving proper export locations?

[benlesh]

@andrewbranch if the onus is on us to make sure that TypeScript/VSCode can find what it needs, that's fine. I just need to know what to do. How do we resolve this on our end? From our perspective, we have to support 3-4 module types, typescript files for source maps, typescript files for types, etc. It's all pretty nuts anyhow. It's not like there's a solid roadmap laid out for how to develop a library that meets everyone's needs.

[benlesh]

FWIW: I agree that the rxjs package is weird. It's ALL weird. I hate module resolution and I'll hate it until CJS dies or ESM gives up.

[benlesh]

What if we exported the types alongside the esm, (literally next to them in the same directories), but kept everything where it is. Does that solve the issue?

[andrewbranch]

rxjs now has the exports set in our package.json. Will that be helpful in resolving proper export locations?

In the future, yes, but right now, unfortunately not.

What if we exported the types alongside the esm, (literally next to them in the same directories), but kept everything where it is. Does that solve the issue?

It at least solves part of the issue, and getting from there to a good UX should be fairly simple. The main issue here is that a module .d.ts file at some path implies that you can import JS at that same path (with a swapped/dropped extension). So removing the types directory, which will never be a valid place to import JS, fixes that problem. After that, you may or may not decide that you want to eagerly reference rxjs/operators from the “root” index.d.ts in order to make those auto-imports available right away. (The tradeoff is that will unnecessarily increase the size of the program for users who have referenced rxjs but not rxjs/operators. I’m not a RxJS user but my impression is that pretty much everyone ends up referenceing rxjs/operators eventually?)

[andrewbranch]

Well, looking more at your package structure, I might need to qualify more. TypeScript currently doesn’t have any way of distinguishing between separate entry points for CJS and ESM. So if you put all your typings alongside the ESM, the paths that we’re going to recognize are going to be rxjs/dist/esm/**/*, which I’m guessing is still not what you want. Assuming your ESM and CJS have compatible types (beware export default), you actually want to put your types in a place that could mimic the structure of your export map. I guess this is what you were doing with the dist/types directory. So maybe a workaround that I didn’t think of before is to leverage the typesVersions package.json field... that actually will make the compiler believe that a subdirectory like dist/types maps to the package root. That’s probably your best bet, on second thought.

Give that a try—if it doesn’t work, I’ll look at it myself next week. (Feel free to DM me on Twitter or in the TypeScript Community Discord.)

[benlesh]

So maybe a workaround that I didn’t think of before is to leverage the typesVersions package.json field...

@andrewbranch so basically something like this?

  "typesVersions": {
    ">=4.1": { "*": ["dist/types/*"] }
  }

[benlesh]

@andrewbranch This change is in rxjs@7.0.0-rc.1 and I'm seeing cases where apps won't compile because the rxjs module cannot be found.

This is the tsconfig:

{
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "experimentalDecorators": true,
    "esModuleInterop": true,
    "isolatedModules": true,
    "strict": true,
    "strictNullChecks": true,
    "noImplicitAny": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "strictFunctionTypes": true,
    "moduleResolution": "node",
    "module": "esnext",
    "noEmit": true,
    "target": "esnext",
    "jsx": "react",
    "allowJs": true,
    "typeRoots": [
      "./types",
      "./node_modules/@types"
    ],
    "types": [
      "jest",
      "node"
    ],
    "skipLibCheck": true,
    "lib": [
      "es2019",
      "dom"
    ]
  },
  "exclude": [
    "./docs",
    "./env/"
  ]
}

[benlesh]

Here's a minimal reproduction. I'm not at all sure what to do about it.
test.zip

[andrewbranch]

Your types field should be index.d.ts, not dist/types/index.d.ts. It gets resolved relative to dist/types/* from typesVersions, so it was trying to find a file at dist/types/dist/types/index.d.ts. (You can see exactly what’s happening via tsc --traceResolution.)

[benlesh]

@andrewbranch that seems to work for from "rxjs" but not for from "rxjs/operators". Meanwhile, I'm trying to organize something to where we can just export everything from "rxjs". But man does this suck a bit. Alternatively, I guess we have to change our entire build to... something? I hate module systems.

[benlesh]

Okay, I suppose what's the ideal setup? CJS next to types? ESM next to types? Assuming that we have to put all of the other stuff in other directories, and we don't want multiple copies of the types.

[andrewbranch]

that seems to work for from "rxjs" but not for from "rxjs/operators"

Are you talking about module resolution or auto imports? I tested importing from "rxjs/operators" before my last comment and it worked as expected. But I didn’t test auto imports. If module resolution works but auto imports do not, that’s a bug.

[benlesh]

Are you talking about module resolution or auto imports

Both, honestly. I couldn't get either working properly.

[benlesh]

If module resolution works but auto imports do not, that’s a bug.

That's the case even before the typesVersions bit. Module resolution has always worked with our package. It's the auto-imports that fail and pull in from the wrong spot.

[andrewbranch]

It's the auto-imports that fail and pull in from the wrong spot.

They were doing exactly what they were expected to do given the structure of the package at the time. Without typesVersions to set a different typings root, the presence of a module declaration file at dist/types/*.d.ts implies the presence of a module implementation file at dist/types/*.js.

I couldn't get either working properly.

The screenshot I posted shows the changes I made to your package.json from 7.0.0-rc.1 alongside module resolution for "rxjs/operators" working correctly, with the exact tsconfig you gave me, using typescript@4.3.0-dev.20210419. Not sure what else to tell you without more info about what you tried.

[benlesh]

So I installed rxjs@7.0.0-rc.2 (which doesn't have the typesVersions), then altered only what you see here (following your screenshot):

After that, when I try to use rxjs, I get this:

I'm sure I'm just misunderstanding what needs done. Sorry, @andrewbranch I'm not trying to be frustrating. haha.

[andrewbranch]

See this comment above:

Your types field should be index.d.ts, not dist/types/index.d.ts. It gets resolved relative to dist/types/* from typesVersions, so it was trying to find a file at dist/types/dist/types/index.d.ts. (You can see exactly what’s happening via tsc --traceResolution.)

[benlesh]

So sorry, I missed that. After making that change, module resolution works again, however, the auto-import still isn't working. Which unfortunately puts us right to where we were before:

[andrewbranch]

It may feel that way, but we are making progress! What you’re seeing now is also expected and can be fixed on your end. @voliva had it right:

Yes, first you need to import something from rxjs/operators in another file of the same project. This usually hints that rxjs/operators also has exports.

Which is the same thing that I was referencing here:

After that, you may or may not decide that you want to eagerly reference rxjs/operators from the “root” index.d.ts in order to make those auto-imports available right away. (The tradeoff is that will unnecessarily increase the size of the program for users who have referenced rxjs but not rxjs/operators. I’m not a RxJS user but my impression is that pretty much everyone ends up referenceing rxjs/operators eventually?)

The problem you’re seeing right now is that dist/types/index.d.ts, with all its imports, never references dist/types/operators/index.d.ts—it only references things in the internal directory—so TypeScript literally has no idea that file exists. You can simply add import './operators' or /// <reference path="operators/index.d.ts" /> to dist/types/index.d.ts to make the compiler aware of that file right away:

(The same topic was also discussed in #30033 (comment))

[benlesh]

Can you think of a way I can test this in an automated way?

[andrewbranch]

Literally testing auto-imports functionality would be hard and out of scope, in my opinion. It should be trivial to test module resolution on a tiny TypeScript program with tsc, though. And you could test the set of files visible to auto-imports by running tsc --listFilesOnly on an input file with content import 'rxjs'. (You could do the same with the compiler’s JS API, but shelling out and splitting STDOUT by line seems easier.)

[benlesh]

Thanks so much for your help, @andrewbranch! The solution did work to get things auto-importing in VS Code properly, however, ultimately we couldn't use this solution because of how it effected tree-shaking with bundlers like Rollup. You can read the fallout here: ReactiveX/rxjs#6276

We're going to have to figure out another solution, which is likely going to involve just getting rid of deep imports, as they've been nothing but a pain.

[andrewbranch]

@benlesh you can use

/// <reference path="./operators/index.ts" />

instead of import "./operators".

[kireet3213]

Disable Path Intellisense. That worked for me.