Update API compatibility testing documentation
rochdev opened this issue · 13 comments
The current documentation says to use this snippet to run API compatibility tests:
const apiCompatibilityChecks = require('opentracing/test/api_compatibility.js');
apiCompatibilityCheck(() => new CustomTracer());However, since version 0.14, it should be instead:
const apiCompatibilityChecks = require('opentracing/lib/test/api_compatibility.js').default;
apiCompatibilityCheck(() => new CustomTracer());See: https://github.com/opentracing/opentracing-javascript#api-compatibility-testing
@yurishkuro Still missing .default since TypeScript doesn't export the default directly.
hm, it worked for me, although bombed on another issue #79, which might be related.
Is there a way to make TypeScript export the default?
Not sure, I know it's also the new behaviour of Babel since version 6. For babel there is a plugin to get the old behaviour back, I don't know about TypeScript. This is related to converting import statements to require, so a quick and easy solution for node is to simply use require directly.
Found a few discussions about this:
I don't see any problems with requiring .default as long as it's documented.
Yeah, I overlooked this in my PR.
Given that README uses const apiCompatibilityChecks = ... (ES6 feature), wouldn't the example be better with import instead of require?
import apiCompatibilityChecks from 'opentracing/lib/test/api_compatibility.js';
I would go after what the NodeJS LTS versions we are targeting support, and they don't support import yet (they do support const, arrow functions, etc). Could be confusing for users if the examples don't run without a transpiler.
.default has actually been part of the CommonJS spec before NodeJS existed so it's not strictly an ES6 concept :)
@felixfbecker @yurishkuro I'm taking a look at open issues... what is the status here? I am not knowledgeable enough about Node to have an opinion, but would like to agree on what we think is best (even if we don't have time to do the work right now).
I think this should be fixed in the README on latest master:
const { apiCompatibilityChecks } = require('opentracing/lib/test/api_compatibility.js'); apiCompatibilityChecks(() => new CustomTracer());
(it destructures all the exports and cherry-picks apiComatibilityChecks)
@felixfbecker SGTM!
See current master (Fix how requiring apiCompatibilityChecks #85 now merged in) :)
The default export in opentracing/lib/test/api_compatibility.js. is a property on the exports object and thus is accessed with .default.
There can be only one default property on the exports object.
This property does not therefore need to be destructured.
closing per #85