dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.24k stars 9.95k forks source link

Support pnpm that installed using a standalone script in SpaProxyLaunchCommand #45700

Open sunghwan2789 opened 1 year ago

sunghwan2789 commented 1 year ago

Given a ASP.NET project with SPA setup:

  <PropertyGroup>
    <SpaRoot>client-app\</SpaRoot>
    <DefaultItemExcludes>$(DefaultItemExcludes);$(SpaRoot)node_modules\**</DefaultItemExcludes>
    <SpaProxyServerUrl>https://localhost:5173</SpaProxyServerUrl>
    <SpaProxyLaunchCommand>pnpm dev</SpaProxyLaunchCommand>
  </PropertyGroup>

SpaProxyLaunchManager failes to launch SPA development server, saying it cannot find 'pnpm.cmd':

info: Microsoft.AspNetCore.SpaProxy.SpaProxyLaunchManager[0]
      No SPA development server running at https://localhost:5173 found.
fail: Microsoft.AspNetCore.SpaProxy.SpaProxyLaunchManager[0]
      Failed to launch the SPA development server 'pnpm dev'.
      System.ComponentModel.Win32Exception (2): An error occurred trying to start process 'pnpm.cmd' with working directory 'C:\project\client-app\'. 지정된 파일을 찾을 수 없습니다.
         at System.Diagnostics.Process.StartWithShellExecuteEx(ProcessStartInfo startInfo)
         at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
         at Microsoft.AspNetCore.SpaProxy.SpaProxyLaunchManager.LaunchDevelopmentProxy()
fail: Microsoft.AspNetCore.SpaProxy.SpaProxyLaunchManager[0]
      Couldn't start the SPA development server with command 'pnpm dev'.
info: Microsoft.AspNetCore.SpaProxy.SpaProxyMiddleware[0]
      SPA proxy is not ready. Returning temporary landing page.

It makes sense because, using a pnpm installation standalone script, we do not have pnpm.cmd, but only pnpm.exe at %localappdata%\pnpm\.

I know I can use pnpm modifying SpaProxyLaunchCommand to have the extension like pnpm.exe dev,

https://github.com/dotnet/aspnetcore/blob/589aa11b5c631ce719e0530d66be8324a6d79169/src/Middleware/Spa/SpaProxy/src/SpaProxyLaunchManager.cs#L169-L174

but it would be great if we could omit it.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

morganbp commented 1 year ago

I can create a pull request on this. Should I just remove that entire if-test then? Or have a specific handling of pnpm command?

sunghwan2789 commented 9 months ago

As a workaround, I could invoke the shell with launch commands:

    <SpaProxyLaunchCommand>pnpm i &amp;&amp; pnpm dev</SpaProxyLaunchCommand>
    <SpaProxyLaunchCommand Condition="$([MSBuild]::IsOsPlatform('Windows'))">cmd.exe /s /c $(SpaProxyLaunchCommand)</SpaProxyLaunchCommand>
    <SpaProxyLaunchCommand Condition="$([MSBuild]::IsOSUnixLike())">sh -c '$(SpaProxyLaunchCommand)'</SpaProxyLaunchCommand>
MattyLeslie commented 5 months ago

Hello @javiercn , would amending the following code: https://github.com/dotnet/aspnetcore/blob/589aa11b5c631ce719e0530d66be8324a6d79169/src/Middleware/Spa/SpaProxy/src/SpaProxyLaunchManager.cs#L169-L176 To something that checks for both .cmd and .exe files within the working directory and then allocating the relevant command.extension be feasible to hopefully future proof this case against .exe files ?

            if (OperatingSystem.IsWindows() && !Path.HasExtension(command))
            {
                var commandWithCmdExtension = $"{command}.cmd";
                var commandWithExeExtension = $"{command}.exe";

                if (File.Exists(Path.Combine(WorkingDirectory, commandWithCmdExtension)))
                {
                    command = commandWithCmdExtension;
                }
                else if (File.Exists(Path.Combine(WorkingDirectory, commandWithExeExtension)))
                {
                    command = commandWithExeExtension;
                }
            }
MattyLeslie commented 4 months ago

Hello @javiercn , would amending the following code:

https://github.com/dotnet/aspnetcore/blob/589aa11b5c631ce719e0530d66be8324a6d79169/src/Middleware/Spa/SpaProxy/src/SpaProxyLaunchManager.cs#L169-L176

To something that checks for both .cmd and .exe files within the working directory and then allocating the relevant command.extension be feasible to hopefully future proof this case against .exe files ?

            if (OperatingSystem.IsWindows() && !Path.HasExtension(command))
            {
                var commandWithCmdExtension = $"{command}.cmd";
                var commandWithExeExtension = $"{command}.exe";

                if (File.Exists(Path.Combine(WorkingDirectory, commandWithCmdExtension)))
                {
                    command = commandWithCmdExtension;
                }
                else if (File.Exists(Path.Combine(WorkingDirectory, commandWithExeExtension)))
                {
                    command = commandWithExeExtension;
                }
            }

Hello @javiercn just looking for a follow up on this. Many thanks