amqp / rhea

A reactive messaging library based on the AMQP protocol
Apache License 2.0
273 stars 80 forks source link

React application based on `create-react-app` cannot include rhea #379

Open jiridanek opened 2 years ago

jiridanek commented 2 years ago

When I upgrade from react-scripts=4.0.3 to react-scripts=5.0.0, I get the following failure from (new version of) webpack:

Compiled with problems:

ERROR in ./node_modules/rhea/lib/connection.js 36:9-22

Module not found: Error: Can't resolve 'os' in '/home/jdanek/repos/qpid/qpid-dispatch/console/react/node_modules/rhea/lib'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "os": require.resolve("os-browserify/browser") }'
    - install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "os": false }

This is a known problem for create-react-app users, https://github.com/facebook/create-react-app/issues/11756

If I am reading the comments correctly, there is an expectation for packages to themselves provide a 'browserified' version of their library. See this commit for handlebars, linked from the create-react-app issue https://github.com/handlebars-lang/handlebars.js/pull/1540/files.

Therefore, I'd like to request a feature in rhea that would just make things work for me again, whatever shape or form such feature needs to take.

grs commented 2 years ago

If I am reading the comments correctly, there is an expectation for packages to themselves provide a 'browserified' version of their library. See this commit for handlebars, linked from the create-react-app issue https://github.com/handlebars-lang/handlebars.js/pull/1540/files.

There is an equivalent file for rhea, also under dist. Can you try adding a similar section to package.json and see if that resolves it for you?

jiridanek commented 2 years ago

The last time I looked, dist/rhea.js contained the var os = require('os'); problematic import. I'll recheck in case nowadays npm install really delivers me browserified version of rhea out of the box.

jiridanek commented 2 years ago

Replacing

import rhea from "rhea";

with

import rhea from "rhea/dist/rhea.js";

silenced the webpack error. So, I don't know why require('os') is not causing problems there, but apparently it really doesn't!

edit: sorry, I started celebrating before I tried to actually run the console:

```this.rhea.websocket_connect is not a function````

I'll investigate more.

jiridanek commented 2 years ago

I'm still confused about the js modules, to tell the truth. As far as I can figure, what rhea needs to provide is an ESM package (as in https://github.com/eslint/rfcs/pull/72/files).

In any case, I haven't figured a way how to import or require the dist/rhea.js in a create-react-app so that it goes through babel/webpack (whatever compilation process react-scripts imposes) so I can use it at the end.

jiridanek commented 2 years ago

I got feedback from @bartoval that the dist/rhea.js is not intended as input to bundling system, but should be included directly from HTML. If I want to continue using rhea in react-scripts, I can use craco or react-rewired to configure the nodejs polyfills in webpack for it. https://github.com/apache/qpid-dispatch/pull/1560#discussion_r855165802

The entire issue probably stems from the fact that rhea is a nodejs library and using it in browser requires some steps (which the new react-scripts` for some reason refuse to perform automatically).

bartoval commented 2 years ago

I think it will be nice providing a further bundle compatible with modules. The current dist/rhea.js or (dist/rhea.min.js) works only using a script tag.

We can create a bundle compatible with UMD using the option --standalone. doc

We just need to modify the command in the package.json and generate both rhea.js and rhea-module.js

"browserify": "browserify -r .:rhea -o dist/rhea.js && browserify -r .:rhea --standalone rhea-umd > dist/rhea-module.js",

then @jiridanek you can call

import rhea from 'rhea/dist/rhea-module'

I can create the PR. What you do think @grs ?

bartoval commented 2 years ago

The entire issue probably stems from the fact that rhea is a nodejs library and using it in browser requires some steps (which the new react-scripts` for some reason refuse to perform automatically).

This is because react-script 5 uses Webpack 5 that removed the node.js polyfill changelog

grs commented 2 years ago

@bartoval, sounds good to me