Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jiridanek opened this issue Feb 12, 2022 · 8 comments
Open

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

jiridanek opened this issue Feb 12, 2022 · 8 comments

Comments

@jiridanek
Copy link
Contributor

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, facebook/create-react-app#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
Copy link
Member

grs commented Feb 13, 2022

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
Copy link
Contributor Author

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 added a commit to jiridanek/qpid-dispatch that referenced this issue Feb 13, 2022
…lement, never rendered directly. Please wrap your <Route> in a <Routes>.

* Error: You cannot render a <Router> inside another <Router>. You should never have more than one in your app.

See https://reactrouter.com/docs/en/v6/upgrading/v5, and therein linked https://gist.github.com/mjackson/d54b40a094277b7afdd6b81f51a0393f

* BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
* Module not found: Error: Can't resolve 'os' in '/home/jdanek/repos/qpid/qpid-dispatch/console/react/node_modules/rhea/lib'
* facebook/create-react-app#11756
* downgraded and filled amqp/rhea#379
@jiridanek
Copy link
Contributor Author

jiridanek commented Feb 13, 2022

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 added a commit to jiridanek/qpid-dispatch that referenced this issue Feb 17, 2022
…lement, never rendered directly. Please wrap your <Route> in a <Routes>.

* Error: You cannot render a <Router> inside another <Router>. You should never have more than one in your app.

See https://reactrouter.com/docs/en/v6/upgrading/v5, and therein linked https://gist.github.com/mjackson/d54b40a094277b7afdd6b81f51a0393f

* BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
* Module not found: Error: Can't resolve 'os' in '/home/jdanek/repos/qpid/qpid-dispatch/console/react/node_modules/rhea/lib'
* facebook/create-react-app#11756
* downgraded and filled amqp/rhea#379
jiridanek added a commit to apache/qpid-dispatch that referenced this issue Feb 17, 2022
…ase (#1517)

* Error: A <Route> is only ever to be used as the child of <Routes> element, never rendered directly. Please wrap your <Route> in a <Routes>.
* Error: You cannot render a <Router> inside another <Router>. You should never have more than one in your app.

See https://reactrouter.com/docs/en/v6/upgrading/v5, and therein linked https://gist.github.com/mjackson/d54b40a094277b7afdd6b81f51a0393f

* BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
* Module not found: Error: Can't resolve 'os' in '/home/jdanek/repos/qpid/qpid-dispatch/console/react/node_modules/rhea/lib'
* facebook/create-react-app#11756
* downgraded and filled amqp/rhea#379
@jiridanek
Copy link
Contributor Author

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
Copy link
Contributor Author

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. apache/qpid-dispatch#1560 (comment)

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
Copy link
Contributor

bartoval commented Apr 25, 2022

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
Copy link
Contributor

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
Copy link
Member

grs commented Apr 26, 2022

@bartoval, sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants