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.4k stars 10k forks source link

Consider providing override for platform detection in JS/TS SignalR client #49065

Open mitchdenny opened 1 year ago

mitchdenny commented 1 year ago

An internal team reached out with a problem they are experiencing with our platform detection logic:

https://github.com/dotnet/aspnetcore/blob/7918436a914ac1cbafa94a359e435c67535c8159/src/SignalR/clients/ts/signalr/src/Utils.ts#L40C9-L40C9

Locally this works, but they are seeing a problem in production where it is detecting that it is in the browser instead of Node. This could possibly be due to global namespace pollution from another package that is impacting their code that runs locally.

They asked whether it would be possible to provide the ability to override the platform detection logic.

Another possibility is that we could see whether there is a better way to detect that we are running in Node vs. looking for the presence of the window global variable.

mitchdenny commented 1 year ago

After looking at the way that Platform.isNode (and friends) are used. I think plumbing through some kind of override from the top level APIs would be pretty ugly with lots of code churn in the library. Given the problem here is global namespace polution - perhaps a little bit more global namespace solution could be our salvation?

What if we allowed someone to set global variables:

signalRPlatformDetectionOverride = "node";

Then Platform.isNode and friends would check this as part of its logic.

josep-llodra commented 1 week ago

Today I needed something like this:

signalRPlatformDetectionOverride = "browser";

Why? I am using Electron and I need it to have nodeIntegration: true and contextIsolation: false.

Hence, Platform.isNode returns true, and require('ws') throws.

So, I manually modified the Platform class to return what I needed and works perfectly.