Skip to content

Auto import not working for lodash #29039

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

Closed
mjbvz opened this issue Dec 15, 2018 · 30 comments
Closed

Auto import not working for lodash #29039

mjbvz opened this issue Dec 15, 2018 · 30 comments
Labels
Domain: TSServer Issues related to the TSServer Duplicate An existing issue was already created

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Dec 15, 2018

From microsoft/vscode#63239

TypeScript Version: 3.3.0-dev.20181214

Search Terms:

  • auto import
  • suggestions / completions

Code

  1. For a simple js project that has lodash installed under node_modules
  2. In a blank js file, type: kebabCase

Expected behavior:
Auto import suggestion to import from lodash returned

Actual behavior:
No suggestion returned

@weswigham weswigham added Domain: TSServer Issues related to the TSServer Needs Investigation This issue needs a team member to investigate its status. labels Dec 17, 2018
@shayded-exe
Copy link

To add to this, it does work once you already have an existing import from lodash in the file. The suggestions don't show up in auto-complete, but it does give a quick fix suggest to add the new method to the existing import from lodash.

@aleclarson
Copy link

This issue provides a repro: microsoft/vscode#67505

@sheetalkamat
Copy link
Member

This issue is duplicate of #28773 wherein during completion we don't have knowledge of symbols not in the program and hence no offering in the completions.

@aleclarson
Copy link

aleclarson commented Jan 30, 2019

@sheetalkamat It's a little surprising that TypeScript doesn't anticipate that a package listed in dependencies will eventually be imported by the program. Is this on the roadmap?

@tnrich
Copy link

tnrich commented Feb 16, 2019

Even when I am using lodash in a file I still don't get auto-import for it. Here is an image showing it not working:
image

Lodash is really the only module I see this not working well for.

@tnrich
Copy link

tnrich commented Mar 10, 2019

Any update on this one? This is definitely something I get annoyed by on a near daily basis. I am just continually surprised that this feature which works so well for most other packages doesn't work for the single most popular js library:

image

Either vscode is doing something wrong or lodash is but it seems really odd that this has been an issue for so long

@tnrich
Copy link

tnrich commented Mar 10, 2019

lodash/lodash#4062

@aleclarson
Copy link

aleclarson commented Mar 10, 2019

@tnrich Do you know of a library (that takes a similar approach for its .d.ts definitions) that works as expected?

@tnrich
Copy link

tnrich commented Mar 10, 2019

@aleclarson I'm not sure how lodash's .d.ts files are defined. Could you elaborate more?

Do lodash's types get pulled from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/lodash by default?

@aleclarson
Copy link

aleclarson commented Mar 10, 2019

@tnrich I dove into @types/lodash to find that export = _ is the issue.

Removing that line is the first step. The problem is that it can't be removed, because
import _ = require('lodash') is probably being used by someone.

The other step is to add an export const line for each Lodash function, though I wonder if something like export * from _.LodashStatic is (or should be) possible. Maybe export as namespace _ is supposed to be doing that?

Not sure if this is an issue with TypeScript itself, or just @types/lodash.

@tnrich
Copy link

tnrich commented Mar 10, 2019

Hmm thanks for diving in @aleclarson. I poked around as well but am not super familiar with typescript so felt a bit lost.

I agree that import _ = require('lodash') is definitely being used by many but there is probably a workaround that allows both.

Is there someone we can ask about this? Maybe one of the @types/lodash maintainers?

@Andy-MS @aj-r -- I couldn't find a list of maintainers but it looks like you both have committed a fair amount to the @types/lodash repo. Could either of you comment on this issue? Thank you 🙏

@aj-r
Copy link

aj-r commented Mar 10, 2019

Yeah lodash is using the namespace 'hack' to support import _ = require('lodash'). I'm not aware of any other way to support that. But on the other hand, I don't think anyone should be using that style of import with modern Typescript (that doesn't mean they aren't though).

@tnrich
Copy link

tnrich commented Mar 11, 2019

@aj-r is the namespace "hack" causing auto-import to fail?

I'm not sure I understand why anyone would be mixing import and require(). That throws lint errors for me.

@aj-r
Copy link

aj-r commented Mar 11, 2019 via email

@aj-r
Copy link

aj-r commented Mar 11, 2019 via email

@tnrich
Copy link

tnrich commented Mar 11, 2019

I just tried lodash-es @aj-r but auto-import doesn't seem to work for me in vscode.

@tnrich
Copy link

tnrich commented Mar 11, 2019

It seems like we're close. It probably has something to do with the .d.ts file. I wonder what needs to be changed to allow auto-import to work correctly..

@aleclarson
Copy link

lodash-es is working on my end. @tnrich Can you provide a repro, and your VSCode/TypeScript version?

While it would be nice if lodash "just worked", I don't think it's possible without losing CommonJS support (which is why lodash-es exists, I assume).

@tnrich
Copy link

tnrich commented Mar 11, 2019

@aleclarson can you share a picture or video of lodash-es auto-import working for you?

@aleclarson
Copy link

lodash-es

@tnrich
Copy link

tnrich commented Mar 11, 2019

@aleclarson sweet thanks for that video! After installing @types/lodash-es it does work for me!

It will unfortunately be somewhat complicated to switch over to using lodash-es in all my packages.

I still think the better solution would be to somehow alter the lodash index.d.ts so that auto import works correctly. As long as import {flatMap} from "lodash"; is a valid import statement it seems like there should be a way

@aleclarson
Copy link

@tnrich When export = is used, a CommonJS module is implied, which means named exports (eg: export const) are no longer allowed (since that's an ESM feature). The problem is that named exports are required for auto-import to work.

When a user writes the following TypeScript:

import { each } from 'lodash'

..it's equivalent to this JavaScript:

const { each } = require('lodash')

..which is "destructuring" the CommonJS exports of the lodash package.

Auto-import can't suggest properties of a CommonJS namespace, because it can't know if the CommonJS package is meant to be used that way.

That being said, you could try opening a new issue with a proposal for export as namespace to be used as a hint that auto-import can suggest the namespace's properties. I could see that being a good idea!

@tnrich
Copy link

tnrich commented Mar 11, 2019

@aleclarson thanks for the detailed explanation. That makes sense to me. Where should I open that new issue? Would that be in the Typescript repo or in a vscode repo?

@aleclarson
Copy link

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Mar 19, 2019
@RyanCavanaugh RyanCavanaugh removed the Needs Investigation This issue needs a team member to investigate its status. label Mar 19, 2019
@RyanCavanaugh
Copy link
Member

Almost every comment in this thread is confused about something and it'd be an hour or more for me to set everything straight here. Please don't read this thread for information.

The primary root cause of this issue is that the JS language service doesn't aggressively include types for packages it can't tell you're using yet, because doing so would be a fairly substantial perf hit for not a lot of gain. Compounding that is that ES6 destructuring imports from CommonJS modules is not a well-defined operation outside of loaders which ignore the spec to make that work.

@aleclarson
Copy link

aleclarson commented Mar 20, 2019

The primary root cause of this issue is that the JS language service doesn't aggressively include types for packages it can't tell you're using yet...

@RyanCavanaugh This comment shows that the actual problem is occurring for a package that is already in use, so what you're saying doesn't seem relevant?

Nevertheless, I want to address your performance claim:

... because doing so would be a fairly substantial perf hit for not a lot of gain.

How often does the dependencies map (in package.json) contain a dependency that isn't intended to be used in the package? Never. I don't see how performance is an issue there, and the gain is substantial enough to complain about...

Compounding that is that ES6 destructuring imports from CommonJS modules is not a well-defined operation outside of loaders which ignore the spec to make that work.

Are you suggesting that @types/lodash shouldn't be using the export as namespace trick? To be clear, that would mean all lodash users must use lodash-es if they want auto-import. I get the feeling that TypeScript doesn't want to encourage CommonJS modules. 😛

@tnrich
Copy link

tnrich commented Aug 1, 2019

Lodash auto import still doesn't work in vscode for me. @RyanCavanaugh could you please suggest some practical steps that we can take to get this fixed?

The primary root cause of this issue is that the JS language service doesn't aggressively include types for packages it can't tell you're using yet, because doing so would be a fairly substantial perf hit for not a lot of gain. Compounding that is that ES6 destructuring imports from CommonJS modules is not a well-defined operation outside of loaders which ignore the spec to make that work.

I don't understand why 80/90% of packages have no issues auto-importing into my project. Seems like lodash is doing something tricky that doesn't quite work. I'd love some more concrete tips that I could try to fix this issue. Thanks!

@ricsam
Copy link

ricsam commented Aug 8, 2019

To just fix the problem I do the following:

yarn add @types/lodash@npm:@types/lodash-es

and maybe yarn add lodash@npm:lodash-es

If you want some functionality from lodash that is not part of lodash es, like runInContext, install it under an alias:

yarn add lodash_original@npm:lodash
yarn add @types/lodash_original@npm:@types/lodash

@tnrich
Copy link

tnrich commented Sep 9, 2019

@ricsam while your suggestion does work brilliantly, it is not compatible with npm. I've run into this issue with it when people try to use my packages:

npm ERR! Invalid dependency type requested: alias

https://stackoverflow.com/questions/54085943/npm-err-invalid-dependency-type-requested-alias

@fhucko
Copy link

fhucko commented Mar 2, 2023

Import in VSC still not working. I have to write _.something to find the method, then delete "_." and manually write import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: TSServer Issues related to the TSServer Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

10 participants