Skip to content

Core 3.1, UseReactDevelopmentServer middleware & Performance #19052

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
yahorsi opened this issue Feb 14, 2020 · 9 comments
Closed

Core 3.1, UseReactDevelopmentServer middleware & Performance #19052

yahorsi opened this issue Feb 14, 2020 · 9 comments
Labels
affected-medium This issue impacts approximately half of our customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-spa ✔️ Resolution: Duplicate Resolved as a duplicate of another issue severity-major This label is used by an internal tool Status: Resolved

Comments

@yahorsi
Copy link

yahorsi commented Feb 14, 2020

Hi Guys,

We recently have started working on the new .NET Core 3.1.1 & React SPA.
In order to start, we have created the app from the template (.NET Core Web Application + React) and noticed that some request that gets proxied to the node takes longer than they should.
The only change in the template we made - we have disabled https redirection and open the local site using just HTTP. As you see on the screenshot, sometimes requests that's get proxied takes about the second and sometimes they are fast. NOTE: Press F5 several times

image

If we run the site using exactly the same command as used by the UseReactDevelopmentServer - npm start, that runs "rimraf ./build && react-scripts start" we will see following timings:

image

Things start working very very fast and that is really noticeable for the developers working on the frontend as frontend needs to reload on the change.

So, my wild guess is that there is something wrong happening in the middleware that is proxying calls to the node.

I have uploaded the app I was using here https://github.com/yahorsi/ReactDevServerMiddlewarePerf

PS1: I have also noticed that it seems when you stop C# app - Node is not stopped and so, after a while you have a LOT's of node instances running
PS2: Seems template needs to be updated as after the start it complains that npm update needed
PS3: Running in the release configuration does not solve the issue

@Pilchie Pilchie added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 14, 2020
@javiercn
Copy link
Member

@yahorsi thanks for contacting us.

We believe this is a duplicate of a similar issue. It has to do with node just listening in IPv4 by default. I believe you can change the redirect middleware to use http://127.0.0.1:<<port>> instead of localhost and that should fix the issue.

@yahorsi
Copy link
Author

yahorsi commented Feb 14, 2020

@yahorsi thanks for contacting us.

We believe this is a duplicate of a similar issue. It has to do with node just listening in IPv4 by default. I believe you can change the redirect middleware to use http://127.0.0.1:<<port>> instead of localhost and that should fix the issue.

Thank you for reply! Few things to clarify

  1. What do you mean by "I believe you can change the redirect middleware to use http://127.0.0.1:<<port>>", I do not see how that could be configured, as not available in the spa.Options or in the UseReactDevelopmentServer call. Could you please point to the sample?
  2. If node must be started some specific way then the template should be fixed as now template made the way that is close to impossible to use due to bad performance?
  3. Could you please point me to the duplicate issue so I can link them and close mine?
  4. As far as I understand there is nothing wrong in using IPv4 and as I noted above when node is started manually it serves pages almost immediately

Thanks

@javiercn
Copy link
Member

@yahorsi You should be able to configure the react dev server to launch on 127.0.0.1 explicitly instead of localhost. See here for details

That should fix the issue.

@yahorsi
Copy link
Author

yahorsi commented Feb 17, 2020

@yahorsi You should be able to configure the react dev server to launch on 127.0.0.1 explicitly instead of localhost. See here for details

That should fix the issue.

This does not work, as, I understand port and host is specified by ReactDevelopmentServer.
So, even if I set REACT_APP_HOST & REACT_APP_PORT env variables in the logs I see:

info: Microsoft.AspNetCore.SpaServices[0]
      Starting create-react-app server on port 56326...

That is corresponding code... BTW, why are you thinking moving node to 127.0.0.1 from localhost will solve the issue?

        private static async Task<int> StartCreateReactAppServerAsync(
            string sourcePath, string npmScriptName, ILogger logger)
        {
            var portNumber = TcpPortFinder.FindAvailablePort();
            logger.LogInformation($"Starting create-react-app server on port {portNumber}...");

            var envVars = new Dictionary<string, string>
            {
                { "PORT", portNumber.ToString() },
                { "BROWSER", "none" }, // We don't want create-react-app to open its own extra browser window pointing to the internal dev server port
            };
            var npmScriptRunner = new NpmScriptRunner(
                sourcePath, npmScriptName, null, envVars);
            npmScriptRunner.AttachToLogger(logger);

            using (var stdErrReader = new EventedStreamStringReader(npmScriptRunner.StdErr))
            {
                try
                {
                    // Although the React dev server may eventually tell us the URL it's listening on,
                    // it doesn't do so until it's finished compiling, and even then only if there were
                    // no compiler warnings. So instead of waiting for that, consider it ready as soon
                    // as it starts listening for requests.
                    await npmScriptRunner.StdOut.WaitForMatch(
                        new Regex("Starting the development server", RegexOptions.None, RegexMatchTimeout));
                }
                catch (EndOfStreamException ex)
                {
                    throw new InvalidOperationException(
                        $"The NPM script '{npmScriptName}' exited without indicating that the " +
                        $"create-react-app server was listening for requests. The error output was: " +
                        $"{stdErrReader.ReadAsString()}", ex);
                }
            }

            return portNumber;
        }

@javiercn
Copy link
Member

See #18062 for the discussion.

@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Feb 18, 2020
@mkArtakMSFT mkArtakMSFT added ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. and removed investigate labels Feb 18, 2020
@ghost ghost added the Status: Resolved label Feb 18, 2020
@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. and removed ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved labels Feb 18, 2020
@mkArtakMSFT mkArtakMSFT modified the milestones: Discussions, Backlog Feb 18, 2020
@mkArtakMSFT
Copy link
Contributor

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

@javiercn javiercn removed their assignment Apr 24, 2020
@ChiefInnovator
Copy link

I am currently writing a book for Azure and some of the samples are based on React and ASP.NET. I am running into this issue, #19052, all the time. Performance is abysmal. Sometimes its instantaneous, others it takes a long time to startup.

The standard ASP.NET React template does not include a good way to exclude the call to the UseReactDevelopmentServer. Instead, it relies on an environment check for env.IsDevelopment and if in development, it makes the call to UseReactDevelopmentServer.

For containers, we would rather use 'npm run build', generate our React template, but still allow debugging of our ASP.NET supporting services. This would allow us just to deploy our static frontend and our dotnet application without deploying NodeJs to our container for development.

The challenge is that the development environment is set in Visual Studio Code when debugging into a container, as it should. Then the call to UseReactDevelopmentServer happens which does not find npm and NodeJs and fails. It is also a spectacular failure too with an uncaught background exception that brings down the entire application. We would expect that this would fail silently.

I have considered not using the ASP.NET React template at all because of these issues and going with a standard Create React App template and use backend set of APIs. For us, we still believe there is significant value with services that are deployed with and support our user interface directly.

The right solution is to compile out the use of the UseReactDevelopmentServer in the container builds, but we cannot do so very easily. You would think that you could, but passing the needed conditionals on 'dotnet build' and 'dotnet publish' is voodoo.
There are many GitHub issues about passing the condition compiles and none that seem promising.

So my ask is the following:

  1. The ASP.NET team improve support and testing of the React template with containers.
  2. Improve the performance of the UseReactDevelopmentServer by changing the React template to use the suggestions of using 127.0.0.1 and/or localhost.
  3. Improve the resiliency of the UseReactDevelopmentServer call so that it does not bring down the application.
  4. A template that includes Docker support that conditionally compiles out the call to UseReactDevelopmentServer.
  5. An update to the Docker extension for Visual Studio Code to add conditional compilation support for ASP.NET applications that incorporate React (or for that matter, any SPA app).

@Brian-Cole
Copy link

For those experiencing this issue and looking for a temporary fix, you can set HOST=::1 in your .env file of your Create React App. This should cause node to listen on IPv6 loopback.

@javiercn javiercn added affected-medium This issue impacts approximately half of our customers severity-major This label is used by an internal tool labels Feb 19, 2021 — with ASP.NET Core Issue Ranking
@javiercn
Copy link
Member

This is a dupe of #22769.

We recommend following the guidance offered there

@javiercn javiercn added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label Oct 28, 2021
@ghost ghost added the Status: Resolved label Oct 28, 2021
@javiercn javiercn removed this from the Backlog milestone Oct 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-medium This issue impacts approximately half of our customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-spa ✔️ Resolution: Duplicate Resolved as a duplicate of another issue severity-major This label is used by an internal tool Status: Resolved
Projects
None yet
Development

No branches or pull requests

6 participants