SignalR npm package not able to be bundled using `esbuild`

[hrueger]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

Hi, I'm using the npm package @microsoft/signalr with version 7.0.4. I'm using esbuild for bundeling, however, it does not correctly resolve some dependencies, like ws.

This is due those hacks:

// 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;
webSocketModule = requireFunc("ws");
eventSourceModule = requireFunc("eventsource");

I don't think this is an issue of esbuild, since it believes that this is a dynamic require. Can we somehow fix this?

I confirmed that this is the issue, by patching the lib to use plain require instead of the requireFunc, then everything works correctly.

Expected Behavior

ws is resolved correctly when using the bundle option of esbuild.

Steps To Reproduce

https://github.com/hrueger/signalr-esbuild-issue

1. yarn install
2. yarn build
3. See __webpack_require__ is still in dist/bundle.js instead of the ws module embedded in the bundle.

Exceptions (if any)

No response

.NET Version

n/a

Anything else?

No response

[BrennanConroy]

Looks like an esbuild issue https://github.com/evanw/esbuild/issues/1921

[hrueger]

Are you sure? I don't think so, to be honest. This is not about esm / cjs, but instead the workaround which is implemented for webpack here, breaks other bundlers (at least esbuild). To those, the call looks like a dynamic require, although it is actually not dynamic. Do you know, what I mean?

[shsuman]

We are facing the same issue with 5.0.17. The workaround was to replace the __non_webpack_require__ with require using webpack plugin.

{
      test: /@microsoft.signalr.dist.cjs.(HttpConnection|FetchHttpClient)\.js/,
      loader: 'string-replace-loader',
      options: {
             search: /__non_webpack_require__/g,
             replace: 'require',
             strict: true
       }
}

The question I want to ask is why is __non_webpackrequire_\ is being used because after searching online, it seems like it is not meant to be used in production code and no other package uses it.

[hrueger]

Thanks!

[BrennanConroy]

Unfortunately this issue will need to be reopened. The fix we made broke other important scenarios so we'll need to find a different way to fix this. Starting with 8.0.7 this esbuild issue will start happening again.

[rahul-sharma-uipath]

@BrennanConroy did it break multiple scenarios or just the esbuild thing?

[sschultze]

For bundling with rollup (for Electron Main), I also have to use an ugly workaround involving the replace plugin. Needless to say that this is very unsatisfactory.

replace({
    preventAssignment: true,
    include: [
        './node_modules/@microsoft/signalr/dist/**/*'
    ],
    values: {
        // @microsoft/signalr 8.0.7
        '/*#__PURE__*/': '',
        'new (requireFunc("tough-cookie")).CookieJar()': 'undefined',
        'this._fetchType = requireFunc("fetch-cookie")(this._fetchType, this._jar);': JSON.stringify(''),
        'requireFunc("ws")': 'undefined',
        'requireFunc("eventsource")': 'undefined'
    },
    delimiters: ['', '']
}),

In addition, the private WebSocket and EventSource properties have to be set in IHttpConnectionOptions manually. And this workaround wouldn't work with cookies.