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

Node standard "os" error #465

Open
santiagoalmeidabolannos opened this issue Jan 27, 2020 · 17 comments
Open

Node standard "os" error #465

santiagoalmeidabolannos opened this issue Jan 27, 2020 · 17 comments

Comments

@santiagoalmeidabolannos

I am getting this The package at "node_modules\zipkin\lib\index.js" attempted to import the Node standard library module "os". it should not be happening.

I add this into my tsconfig.json

"paths": { ... "node-fetch": [ "node_modules/empty-module/index.js" ], "os": [ "node_modules/empty-module/index.js" ] },

@jcchavezs
Copy link
Contributor

Could be related to b3b5a0c? cc @jdb8

@santiagoalmeidabolannos
Copy link
Author

exactly, I remove those lines and it works

@jdb8
Copy link
Contributor

jdb8 commented Jan 27, 2020

Which version of node is producing this error? Can you paste the full traceback if there's more? I can't tell what's actually throwing the error here.

@santiagoalmeidabolannos
Copy link
Author

It is simple, I am running this on React, no Node standard library runs on it but base on the package description it shouldn't happen

@jdb8
Copy link
Contributor

jdb8 commented Jan 27, 2020

Oh, this is running in a browser environment? I wonder if we need to wrap the import-level execution in a check for that?

@santiagoalmeidabolannos
Copy link
Author

doesn't look bad. Maybe this is helpful https://github.com/flexdinesh/browser-or-node#readme, without any extra package this can be used:
var isBrowser=new Function("try {return this===window;}catch(e){ return false;}");
or
var isNode=new Function("try {return this===global;}catch(e){return false;}");

@jdb8
Copy link
Contributor

jdb8 commented Jan 27, 2020

@santiagoalmeidabolannos just for additional clarification, do you see this error in the browser console at runtime or in a warning at build-time? Just curious for future reference, and also to understand whether this breaks the build or causes errors or something else.

@santiagoalmeidabolannos
Copy link
Author

santiagoalmeidabolannos commented Jan 27, 2020

@jdb8 I am using React Native with Expo to be exact, I get the error when Expo is building the bundle. I guess the same will happen during the build process (prod or dev server) on any modern JavaScript framework.

@jcchavezs
Copy link
Contributor

Any chance any of you can come up with a fix for this?

@santiagoalmeidabolannos
Copy link
Author

I am not sure which of your modules are currently using Node standard library modules

@jcchavezs
Copy link
Contributor

Ping @jdb8

@jdb8
Copy link
Contributor

jdb8 commented Feb 4, 2020

So if I understand correctly, this issue is happening because the code is being imported outside of a node environment: and my change introduced an import-time side effect which assumes the environment is node.

@jcchavezs are there any pre-existing tests for this case? I wrote my code with the assumption that it would only be called in node, but if this package supports other environments, it would be worth setting up an integration test for this + any other import issues.

Please correct me if I'm missing any context here.

@santiagoalmeidabolannos
Copy link
Author

This is the first sentence of the README file 😊 This is a set of libraries for instrumenting Node.js and browser applications. The zipkin library can be run in both Node.js and the browser.

@jcchavezs
Copy link
Contributor

jcchavezs commented Feb 4, 2020 via email

@santiagoalmeidabolannos
Copy link
Author

santiagoalmeidabolannos commented Feb 5, 2020

That's great but I still can't use the latest version of your package. A PR is needed to make the usage/import if node standard modules optional depending on the environment.

@jdb8 can you help with that?

@jdb8
Copy link
Contributor

jdb8 commented Feb 6, 2020

@jcchavezs @santiagoalmeidabolannos I don't have a ton of time at the moment to work on a fix for this, but it sounds like the problem is well-understood (needing to wrap this in an environment check).

My question around the test was mainly to work out if there's an existing CI setup to run the code in a browser or react-native environment, since presumably this would be caught by that. I'm assuming such a test suite doesn't exist otherwise my change would have failed CI.

@Ruwiseturtle
Copy link

Android Bundling failed 8902ms (E:\programm files\GITHUB\React_native-firstProject\node_modules\expo\AppEntry.js)
The package at "node_modules@expo\metro-config\build\ExpoMetroConfig.js" attempted to import the Node standard library module "os".
It failed because the native React runtime does not include the Node standard library.
Learn more: https://docs.expo.dev/workflow/using-libraries/#using-third-party-libraries уже неделю с этим разбираюсь. Что это за ошибка? как ее исправить?

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

No branches or pull requests

4 participants