search

opentracing / opentracing-javascript

OpenTracing API for Javascript (both Node and browser). 🛑 This library is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io
Apache License 2.0
1.09k stars 108 forks source link

Default import is not working in some contexts #132

Open mlarcher opened 5 years ago

mlarcher commented 5 years ago

I am running some code containing ES modules (in .mjs files) in a docker container with node:12.10.0 and the --experimental-modules flag. I am able to import opentracing from 'opentracing'; and things work as expected.

The issue appears when testing the code :

I am using Jest and therefore need to transpile the code through babel-jest. The setup I have works well, except for opentracing. In that case, 'opentracing' ends up being undefined, and things like opentracing.Tags result in an error.

I'm not sure exactly what's going wrong, but it seems to be related to the default export handling :

If I comment out the Object.defineProperty(exports, "__esModule", { value: true }); line in lib/index.js of opentracing, things work well.

They work correctly too if I add a exports.default = exports; in that file.

Without touching the opentracing node_module, I was able to make things work in the test environment by calling import { Tags } from 'opentracing';, but that does not work in the production environment (i.e. no transpilation process).

Please let me know if I can provide additional information to dig into the issue.

felixfbecker commented 5 years ago

The package doesn't have a default export: https://github.com/opentracing/opentracing-javascript/blob/master/src/index.ts so a default import is not expected to work.

The correct import would be:

import * as opentracing from 'opentracing'

does that not work either?

felixfbecker commented 5 years ago

I am not familiar with how --experimental-modules work. Can it import CommonJS modules at all?

mlarcher commented 5 years ago 
import * as opentracing from 'opentracing'

works in testing environment, but not in node environment. In node, opentracing ends up being an object with a default key wrapping all the exports, so everything breaks. On the contrary, that key is not present when using babel in the tests. I didn't find a way to have them both work so far.

mlarcher commented 5 years ago

And --experimental-modules can import CommonJs, yes. See https://nodejs.org/api/esm.html#esm_enabling

felixfbecker commented 5 years ago

This sounds more like a problem of general Node/Babel/TypeScript ESM support interoperability, not anything specific to OpenTracing.

Is it not possible to run Jest with --experimental-modules? That seems like a deficiency of Jest too

mlarcher commented 5 years ago

Jest doesn't allow passing flags to node, and they won't support modules until they get out of experimental mode (expected for this october when node 12 goes LTS). I agree that it would be the best solution, though.

I believe it's an issue with opentracing because that's the only module that gives me this problem, all other work fine even when compiled through babel for testing purposes. The decision to pursue it or close the ticket is yours to make.

On my part, after experimenting I decided that ES modules support was not good enough yet to use them in the backend codebase, and reverted all the changes.

felixfbecker commented 5 years ago

Do you use any other modules that are compiled with TypeScript (tsc) and work correctly? Then we could do the same