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

circular reference cause inherits() became empty object #80

Open
normanzb opened this issue Jul 12, 2023 · 6 comments
Open

circular reference cause inherits() became empty object #80

normanzb opened this issue Jul 12, 2023 · 6 comments

Comments

@normanzb
Copy link

normanzb commented Jul 12, 2023

the line below refs to inherits() from inherits module, but inherits module reference back to util module by checking the implementation on util module first, causing the first require('inherits') to returns empty object (default exports is empty object), if then util.inherits() gets call immediately it will throw.

problematic line in question:
https://github.com/browserify/node-util/blame/ef984721db7150f651800e051de4314c9517d42c/util.js#L592

the line needs to be updated to require('inherits/inherits_browser')

@ljharb
Copy link
Member

ljharb commented Jul 12, 2023

I don't see why - the inherits package has a "browser" field which bundlers should be handling automatically, and in node it would need to be "inherits" directly.

@normanzb
Copy link
Author

@ljharb wow that's quick reply, we are using it in react native, neither node nor browser, there are potentially more js runtime out there...

@ljharb
Copy link
Member

ljharb commented Jul 12, 2023

Sure, that's fine - metro should either be able to handle circular references or it should use the browser endpoint, since RN is much more browser-like than node-like.

This is a node module, so it's up to transformation tools to handle using it in anything other than node (altho "browsers" are especially important).

@normanzb
Copy link
Author

normanzb commented Jul 12, 2023

thx for the reply, 2 cents of my understanding:

This is a node module

this is a module that mimic a node module, not the node module itself.
the true util is a built in nodejs module, it is unlikely anyone would purposefully install this polyfill package in a node env -- if they do most likely is because the lib was included as a part of a dep. most likely the reason this lib is running is because it is not a node env hence no built-in util. based on that context having another check of require('util') seems redundant to me, it either ref back to this lib or this lib competely no need to be ran at all.

it should use the browser endpoint

just fyi, react native is far from the browser, it doesn't have dom api, it doesn't have the globals available in browser and there is no regulation and authorities that control what is available in react nataive.

so unfortunately there is no golden rule or silver bullet in terms of which platform to use, package support is difficult to find, but RN is a growing community, i wish the maintainer of this lib do consider support of it in the future.

@ljharb
Copy link
Member

ljharb commented Jul 12, 2023

ha, true enough :-) this is a node module intended for transparent replacement by a bundler for browser usage.

Either way, supporting circular references are required to support CJS as well as ESM, so that seems like the underlying problem here for metro.

@Richienb
Copy link

It looks like Webpack is also affected

Richienb/node-polyfill-webpack-plugin#57

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