Skip to content

Load 'node-pty' async (fix #105345)#105401

Merged
bpasero merged 2 commits intomasterfrom
ben/105345
Aug 26, 2020
Merged

Load 'node-pty' async (fix #105345)#105401
bpasero merged 2 commits intomasterfrom
ben/105345

Conversation

@bpasero
Copy link
Copy Markdown
Member

@bpasero bpasero commented Aug 26, 2020

This PR fixes #105345

@bpasero bpasero added this to the On Deck milestone Aug 26, 2020
@bpasero bpasero requested a review from Tyriar August 26, 2020 08:28
@bpasero bpasero self-assigned this Aug 26, 2020
@bpasero bpasero mentioned this pull request Aug 26, 2020
const args = shellLaunchConfig.args || [];
this._logService.trace('IPty#spawn', shellLaunchConfig.executable, args, options);
const ptyProcess = pty.spawn(shellLaunchConfig.executable!, args, options);
const ptyProcess = (await import('node-pty')).spawn(shellLaunchConfig.executable!, args, options);
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar Aug 26, 2020

Choose a reason for hiding this comment

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

Looks good, can you also change the import to import type to make sure we don't accidentally import the code again?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nvm, I did it 🙂.

@Tyriar Tyriar modified the milestones: On Deck, August 2020 Aug 26, 2020
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Good to merge after CI compile

@bpasero bpasero merged commit 995302f into master Aug 26, 2020
@bpasero bpasero deleted the ben/105345 branch August 26, 2020 13:46
@bpasero
Copy link
Copy Markdown
Member Author

bpasero commented Aug 26, 2020

@Tyriar oh nice, learnt something (import type...)

@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2020
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 this pull request may close these issues.

Load 'node-pty' async

2 participants