Aspnetcore: Signalr with Angular SSR: Cannot find module 'request'

[rceraline]

Describe the bug

When using SignalR with Angular in SSR it throws an error. It used to be the error describe in this issue https://github.com/aspnet/AspNetCore/issues/7823 but after updating to version 1.1.4, the error is now the following:

NodeInvocationException: Prerendering failed because of error: Error: Cannot find module 'request' at Function.Module._resolveFilename (internal/modules/cjs/loader.js:580:15) at Function.Module._load (internal/modules/cjs/loader.js:506:25) at Module.require (internal/modules/cjs/loader.js:636:17) at require (internal/modules/cjs/helpers.js:20:18) at Object../node_modules/@aspnet/signalr/dist/cjs/NodeHttpClient.js

To Reproduce

Steps to reproduce the behavior:

1. Create a project with asp.net core & Angular 7
2. Configure SSR
3. Use @aspnet/signalr 1.1.4

[anurse]

Can you post a sample project that reproduces the issue? The app we used when testing #7823 works when targeting 1.1.4 so something must be different here. Even if it's just a matter of creating a project with the default template, having a project we can directly run lets us compare with our test code.

[rceraline]

Hi @anurse, I did what you suggested and was actually able to make it work in a sample project. I still don't understand why it is not working on my initial project. I need more time to figure out why this is happening. Thanks.

[rceraline]

Hi @anurse, after some testing, I was able to reproduce the error in a sample project. https://github.com/rceraline/signalrapp

It happens when we activate SSR and bundle all dependencies. See angular.json file bundleDependencies all. I use that feature to not have to publish the node_modules folder in production.

"server": {
          "builder": "@angular-devkit/build-angular:server",
          "options": {
            "outputPath": "dist-server",
            "main": "src/main.server.ts",
            "bundleDependencies": "all",
            "tsConfig": "src/tsconfig.server.json"
          }
        }

To reproduce the issue:

• run these commands

git clone https://github.com/rceraline/signalrapp
cd signalrapp\SignalRApp\ClientApp
npm i
npm run build
npm run build:ssr
cd ..
dotnet run

• rename or delete node_modules folder in signalrapp\SignalRApp\ClientApp.
• navigate to https://localhost:5001

[stephenlautier]

I had just tested my other issue https://github.com/aspnet/AspNetCore/issues/7823, it seems to be working fine (didnt test a fully working app YET tho).

But seeing your code, I think the only issue is because you are using connection.start(), not exactly sure what you're trying to do, but my suggestion is - don't connect to sockets in SSR.
As most probably even if it works, it won't still work correctly e.g. won't wait for data to load.. and well server shouldn't wait for an event to happen either

That said ideally this should still be fixed if its an issue, otherwise node apps bundled won't work i guess

[rceraline]

Hi @stephenlautier, I may have called connection.start() in the sample but I don't think this is the problem. In my real application the start is in a lazy loaded module which is not executed in SSR.

Did you try to execute your code with bundleDependencies set to all? As I explained in my previous message, the issue occurs only when bundleDependencies in the angular.json file is set to all. Then by removing the node_modules folder we can see the problem.

[rceraline]

After looking at the NodeHttpClient.js file, I think the problem occurs because of this

let requestModule: Request.RequestAPI<Request.Request, Request.CoreOptions, Request.RequiredUriUrl>;
if (typeof XMLHttpRequest === "undefined") {
    // In order to ignore the dynamic require in webpack builds we need to do this magic
    // @ts-ignore: TS doesn't know about these names
    const requireFunc = typeof __webpack_require__ === "function" ? __non_webpack_require__ : require;
    requestModule = requireFunc("request");
}

When we bundle all dependencies (angular config), node_modules folder can be deleted and request module is now missing.
Before fixing https://github.com/aspnet/AspNetCore/issues/7823, it was the same logic but for NodeHttpClient:

nodeHttpClientModule = requireFunc("./NodeHttpClient");

When we don't bundle all dependencies, since request module is present in node_modules, there is no problem. I guess that in order to make it work in both scenarios, we'll have to always import request module instead of trying to bypass webpack dynamic require.

[anurse]

Calling connection.start() in pre-rendering is definitely problematic in a few ways, since it will trigger separate outbound TCP connections from your server, back to itself, for each request. This means that each client request will trigger the request and you can easily run out of client-side ports.

[rceraline]

Hi @anurse, I already explained that the issue here is not connection.start() been called in pre-rendering. Actually in the sample that I provided, it is not even called unless you, on purpose, navigate to '/test' because it is in a lazy loaded module. In my real application (not the sample), I excluded some paths from pre-rendering then it won't be pre-render. But, to remove any confusion, I removed completely any call to connection.start from the sample. Again, the issue here is not connection.start.

If we look at my last messages, we can see that the problem appears before any call. It happens when we bundle all dependencies without including the node_modules to the publish folder. I described the repro steps ealier.

[stephenlautier]

@rceraline hmm ok.. not sure, mine seems to work after the update if you wanna take a look, see this comment to run my sample project https://github.com/aspnet/AspNetCore/issues/7823#issuecomment-466542348.

Mine should also be bundled etc... To be clear I had tested again and in SSR I saw some resolve url error, but the SSR still worked and html was still being generated.

[rceraline]

@stephenlautier after I copied past the code (https://github.com/aspnet/AspNetCore/issues/10400#issuecomment-496664468), I thought it was obvious that the request module is not imported and webpack does not bundle it. Exactly like it was for NodeHttpClient.

I did clone your repo and as expected, after launching your app, as soon as I removed request module from the node_modules folder, I received the same error. When we bundle all dependencies, node_modules should not be used anymore. The goal is to not have to deploy node_modules folder in production. I guess a workaround could be importing manually the request module from my code since the signalr client does not do it.

[anurse]

So it looks like the problem here steps from the fact that we are intentionally trying to auto-detect a Node scenario and dynamically import request. Since we build with webpack, we have to do this to stop our own build from bundling the entire request module in our own build. It seems like that is likely to be interfering with your webpack build.

We may have to go back to the drawing board on how to resolve this issue without breaking other webpack builds.

[rceraline]

@anurse thank you, now we understand each other. So, I get that you don't want to bundle the request module for "browser builds" but since you use it in a node environment, it is a dependency so it has to be bundled no? Otherwise maybe 2 different builds: 1 for node, 1 for browser?

[anurse]

It's not anywhere near as common, in my experience, to bundle packages in for a node application. Generally node apps directly load their components out of node_modules. That's not always the case, but in my (very limited) experience it has been much more common. The tricky thing here is that when you are using SSR, you have a browser application that is running inside node on the server. This messes with our detection logic.

Otherwise maybe 2 different builds: 1 for node, 1 for browser?

We basically have that already, we have the dist/cjs folder inside the package for Node and module loaders like WebPack and the dist/browser folder inside the package for <script> tags. The problem is that we have some overlapping scenarios that we haven't considered. There are two "axes" here. Across the top is the runtime, and down the left is how you want to consume SignalR.

| | Node.js | Browser |
| - | - | - |
| Single file (dist/browser, or WebPack) | A | B |
| Modules (dist/cjs) | C | D |

Right now, we don't support box A. If you're bundling everything up into a single file, we expect you to only be using that file on Node.js the browser. So we don't make any effort to bundle in Node-only dependencies like request. Server-side rendering is the tricky part here. When using server-side rendering, we are running your single-file bundle (produced by WebPack) in Node.js, so we are effectively in that unsupported box A above.

Since we only want to load request it in Node.js, we use the __non_webpack_require__ function to force our own WebPack build to ignore it and leave the require call in. This works fine in box C, since you have the node_modules directory to work from. But, as you observed, it doesn't work in box A (which is precisely where SSR operates).

One simple way we could fix this is by "lazily" loading request. If you never call connection.start() there's no reason we need to actually load it. We load it eagerly right now, at the top of the file, which means it always has to try to resolve it when running in Node.js.

EDIT: I mixed up "Node.js" and "browser" in one section. The table is still accurate. Updated inline.

[rceraline]

@anurse thank you for the good explanation.

One simple way we could fix this is by "lazily" loading request. If you never call connection.start() there's no reason we need to actually load it. We load it eagerly right now, at the top of the file, which means it always has to try to resolve it when running in Node.js.

The load would be then executed when we call connection.start()? That should work for me.

[anurse]

Triage: We don't have the capacity to get this done for 3.0 but think it's important. Putting in the backlog to bring up again in 3.1 planning.

[CarlosTorrecillas]

@anurse , do you have an aprox ETA for this? Just to know when v3.1 will be available. It's a blocker for us and would like to plan a new version with the fix included

[anurse]

3.1 is scheduled for release in November, as per the .NET Core Roadmap.

[CarlosTorrecillas]

Thanks for the info - till then, is there any workaround for this? Say that because I tried to manually import the 'request' module into my lazy loaded module containing all the SignalR stuff and unfortunately it didn't work. I'm not sure if you guys have a sample of how to fix this in the meantime? Thanks again!

[rceraline]

Hi @CarlosTorrecillas, since all dependencies are not bundled, in my case I decided to add an extra step in my build pipeline. I do a npm i eventsource request ws right before deploying my app from the output folder. Like that there will be a node_modules folder deployed with only the missing dependencies. I realized that it is not only the request package missing but also eventsource & ws.

[CarlosTorrecillas]

I see @rceraline ... thank you so much for this!

[grandchamp]

Is there a workaround for this except changing the build pipeline?

[CarlosTorrecillas]

In my case I added it as part of building the docker image which is essentially the same stuff

[BrennanConroy]

Fixed via https://github.com/dotnet/aspnetcore/pull/19832