search

pnp / vscode-viva

With the SharePoint Framework Toolkit extension, you can create and manage your SharePoint Framework solutions on your tenant. All actions you need to perform during the development flow are at your fingertips.
https://marketplace.visualstudio.com/items?itemName=m365pnp.viva-connections-toolkit
MIT License
34 stars 14 forks source link

🐞 Bug report: Terminal receives "pm i" instead of "npm i" when choosing npm installation option #246

Open Saurabh7019 opened 3 weeks ago

Saurabh7019 commented 3 weeks ago

⭐ Priority

(Low)☹️ Something is a little off

📝 Describe the bug

Creating a new project and choosing npm i after the project is created doesn't work as expected. The terminal receives pm i instead of npm i.

👣 Steps To Reproduce

  1. Create a new project.
  2. Provide the necessary project details.
  3. Select the option to 'Run npm i after the project is created'.
  4. Observe the terminal output to see if 'npm i' command is executed correctly.

📜 Expected behavior

The terminal should receive npm i when the project is created.

📷 Screenshots

image

❓SharePoint Framework Toolkit version

3.2.1

❓Node.js version

v18.17.1

🤔 Additional context

No response

nicodecleyre commented 2 weeks ago

Hi @Saurabh7019,

Been testing this one with the latest version 3.2.1 and with node.js version 18.17.1, but getting npm i when a new project is created. image

Can you double check if there is terminal.sendText('npm i'); in src/extension.ts?

Adam-it commented 2 weeks ago

@nicodecleyre thank you jumping in on this. You Rock 🤩 Basically this error 'might not always happen' 😉. I know seems strange but I will explain why. How I developed it is that is not waiting for terminal to show image

So I create a new terminal instance and send a text to it without waiting for it to 'boot up'. VS Code terminal is integrated so if someone has the terminal customized (for example uses POSH in PS terminal) it might take a bit some time for new terminal to show. From what I am aware the sendText method works as a stream so some part might be send but not 'recieved' by the terminal in time as it was not up yet. This is my bad and I did this implementation totally wrong assuming everything will go without failures.

What we should do is do something like it is done in runCommand method in TerminalCommandExecuter.ts

Here not only I create the terminal but wait for it to show before I pass the command.

Ideally what I was thinking of this fix was to reuse the TerminalCommandExecuter in the extension.ts to unify the way we pass commands to terminal in all places and make it more bulletproof.

Saurabh7019 commented 2 weeks ago

Hi @Saurabh7019,

Been testing this one with the latest version 3.2.1 and with node.js version 18.17.1, but getting npm i when a new project is created. image

Can you double check if there is terminal.sendText('npm i'); in src/extension.ts?

I have customized my PowerShell prompt using PSReadLine and included the CLI.Microsoft365.PowerShell.Predictor module in my startup profile. This customization is causing a significant delay in loading my personal and system profiles, which could be why I am consistently reproducing the issue.

You might not experience this issue even with the same customization because of your lightning-fast machine. I remember your CLI tests execution times were impressively quick. :)

Adam-it commented 2 weeks ago

You might not experience this issue even with the same customization because of your lightning-fast machine. I remember your CLI tests execution times were impressively quick. :)

Running CLI tests in light speed be like StarWarsLightGIF