zipkin-js icon indicating copy to clipboard operation
zipkin-js copied to clipboard

Node standard "os" error

Open santiagoalmeidabolannos opened this issue 5 years ago • 17 comments

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" ] },

santiagoalmeidabolannos avatar Jan 27 '20 15:01 santiagoalmeidabolannos

Could be related to https://github.com/openzipkin/zipkin-js/commit/b3b5a0ca7466f76e9139f2f9409d5af4f42ea3f7? cc @jdb8

jcchavezs avatar Jan 27 '20 20:01 jcchavezs

exactly, I remove those lines and it works

santiagoalmeidabolannos avatar Jan 27 '20 20:01 santiagoalmeidabolannos

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.

jdb8 avatar Jan 27 '20 20:01 jdb8

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

santiagoalmeidabolannos avatar Jan 27 '20 20:01 santiagoalmeidabolannos

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

jdb8 avatar Jan 27 '20 20:01 jdb8

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;}");

santiagoalmeidabolannos avatar Jan 27 '20 21:01 santiagoalmeidabolannos

@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.

jdb8 avatar Jan 27 '20 21:01 jdb8

@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.

santiagoalmeidabolannos avatar Jan 27 '20 21:01 santiagoalmeidabolannos

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

jcchavezs avatar Jan 28 '20 19:01 jcchavezs

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

santiagoalmeidabolannos avatar Jan 29 '20 10:01 santiagoalmeidabolannos

Ping @jdb8

jcchavezs avatar Feb 03 '20 22:02 jcchavezs

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.

jdb8 avatar Feb 04 '20 00:02 jdb8

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.

santiagoalmeidabolannos avatar Feb 04 '20 09:02 santiagoalmeidabolannos

@jdb8 will this work as an example? https://github.com/openzipkin/zipkin-js-example/tree/master/react-native-example

jcchavezs avatar Feb 04 '20 11:02 jcchavezs

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?

santiagoalmeidabolannos avatar Feb 05 '20 12:02 santiagoalmeidabolannos

@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.

jdb8 avatar Feb 06 '20 01:02 jdb8

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 уже неделю с этим разбираюсь. Что это за ошибка? как ее исправить?

Ruwiseturtle avatar Apr 30 '24 06:04 Ruwiseturtle