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

Check auto-import completion for spread assignment #56247

Merged
merged 5 commits into from
Dec 8, 2023

Conversation

hanzooo
Copy link
Contributor

@hanzooo hanzooo commented Oct 29, 2023

Fixes #56071

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Oct 29, 2023
@hanzooo hanzooo changed the title Enhancement for spread assignment auto-import completion Check auto-import completion for spread assignment Oct 31, 2023
src/services/completions.ts Outdated Show resolved Hide resolved
@sandersn sandersn requested a review from iisaduan November 20, 2023 23:00
@sandersn sandersn removed their assignment Nov 20, 2023
@hanzooo hanzooo requested a review from sandersn December 6, 2023 14:55
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case for

const test7: { foo: string } = { foo/*7*/ }

I think we don’t expect an auto-import there.

@hanzooo
Copy link
Contributor Author

hanzooo commented Dec 7, 2023

Can you add a test case for

const test7: { foo: string } = { foo/*7*/ }

I think we don’t expect an auto-import there.

I tried this test case and it can make an auto-import. Maybe the type of { foo: string } is not considered here?

@hanzooo hanzooo requested a review from andrewbranch December 7, 2023 14:38
@andrewbranch
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 7, 2023

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at 1ed01ac. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 7, 2023

Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/159011/artifacts?artifactName=tgz&fileId=4FA1CE711404ACA8317BC9A7004C66F35F96B29C265EB006DC22D6371185572A02&fileName=/typescript-5.4.0-insiders.20231207.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

@andrewbranch
Copy link
Member

I tried this test case and it can make an auto-import.

Hm, I just tried it locally and I don’t get an auto-import. Can you add the test so I can see what’s going on?

@hanzooo
Copy link
Contributor Author

hanzooo commented Dec 8, 2023

I tried this test case and it can make an auto-import.

Hm, I just tried it locally and I don’t get an auto-import. Can you add the test so I can see what’s going on?

Yeah, sure. Seens that the test case isn't right. I changed it, it should works now.

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hanzooo!

@andrewbranch andrewbranch merged commit 96e7af4 into microsoft:main Dec 8, 2023
19 checks passed
c0sta pushed a commit to c0sta/TypeScript that referenced this pull request Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

No import suggestion when destructuring inside object inside array
6 participants