Explicit reference to Buffer class (node)

GaelBi commented 1 year ago

Hello,

Working with Angular 16, I met the following error :

ReferenceError: Buffer is not defined
    at serializeSetupFrame (Codecs.js:315:11)
    at serializeFrame (Codecs.js:227:20)
    at WebsocketDuplexConnection.send (WebsocketDuplexConnection.js:86:55)
    at ClientServerInputMultiplexerDemultiplexer.send (ClientServerMultiplexerDemultiplexer.js:144:23)
    at RSocketConnector.<anonymous> (RSocketConnector.js:146:80)

I had to expose in the global scope manually the reference to the Buffer class. But, like it is mentioned in the suggestion from the official documentation, maybe, it would be preferable to set an explicit reference of this class when it is used ?

The official documentation : https://nodejs.org/api/buffer.html#buffer "While the Buffer class is available within the global scope, it is still recommended to explicitly reference it via an import or require statement."

Desired solution

Explicit import of Buffer when it is required (like in Codecs.ts). import { Buffer } from 'node:buffer';

viglucci commented 1 year ago

Hey @GaelBi, thanks for the feedback.

Can you let me know which version of RSocket you were using? Was it 0.0.27 or 1.0.0-alpha? I assume 1.0.0-alpha since you reference Codecs.ts.

Either way; as you've noted, when building a client side application there are some challenges with the buffer module. I believe you would of encountered this issue even if we explicitly import Buffer as your link suggests, since the nodejs Buffer module would not be compatible with Browser environvments (as far as I understand).

Given that, this is why providing a Buffer polyfill is important. Currently we expect users of RSocket in client side applications to configure this themselves since it is dependent on the bundler and other technology choices that the application authors have made.

If we drop support for older Node versions we may import Buffer directly as you've suggested, however I don't believe it would have resolved your problem in the browser environment with Angular.

Let me know if that clears things up.

GaelBi commented 1 year ago

Hi @viglucci ,

Thanks for your answer. I am using the 1.0.0-alpha.3 version. No, I think the issue should be resolved with this import. Indeed, the buffer module seems to be now compatible with Browser environment, my solution has been to import and expose it in window context (without any polyfill).

The polyfills seem to be less and less used, for exemple, we can read in the documentation of webpack 5 : "...the module landscape changed and many module uses are now written mainly for frontend purposes..." https://webpack.js.org/blog/2020-10-10-webpack-5-release/#automatic-nodejs-polyfills-removed

Regards, Gaël

viglucci commented 1 year ago

Hey @GaelBi, thanks for the additional comment.

I'm happy to give this a try and see if importing from 'node:buffer' produces a bundle which is both browser compatible and doesn't require an explicit Pollyfill.

Alternatively, if you'd be interested in submitting a branch doing so I would be more than happy to review the results there as well.

rsocket / rsocket-js

Explicit reference to Buffer class (node) #267

Desired solution