gotwarlost / istanbul

Yet another JS code coverage tool that computes statement, line, function and branch coverage with module loader hooks to transparently add coverage when running tests. Supports all JS coverage use cases including unit tests, server side functional tests and browser tests. Built for scale.
Other
8.7k stars 786 forks source link

Fix circular dependency found by Node.JS 14 #940

Open robertkiel opened 3 years ago

robertkiel commented 3 years ago

Istanbul version 0.4.5 seems to produce a circular dependency when using Node.JS 14

(node:23084) Warning: Accessing non-existent property 'VERSION' of module exports inside circular dependency
    at emitCircularRequireWarning (internal/modules/cjs/loader.js:650:11)
    at Object.get (internal/modules/cjs/loader.js:664:5)
    at Object.<anonymous> (/home/robert/Documents/HOPR/hoprnet/node_modules/sc-istanbul/lib/command/help.js:9:37)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at /home/robert/Documents/HOPR/hoprnet/node_modules/sc-istanbul/lib/util/factory.js:56:35

The problem seems to be that VERSION is loaded before lib/commands/index.js is built and ready to be exported.

Requiring VERSION once it is needed, namely once the help command runs, solves the problem.

Referring to issue https://github.com/hoprnet/hoprnet/issues/1241.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 97.523% when pulling e735f69e55856b4b3c55638b34cd40b87f55bc46 on robertkiel:master into bc84c315271a5dd4d39bcefc5925cfb61a3d174a on gotwarlost:master.

sshmaxime commented 3 years ago

Same error on Node v.15.5.0. thx for the PR, hoping it will get merged asap

thatjs commented 2 years ago

A simpler change is just to read the VERSION from Command defined in line 6:

var Command = require('./index.js'),
...
    // VERSION = require('../../index').VERSION
    VERSION = Command.VERSION,

👍 Can we get this change merged?

cowwoc commented 1 year ago

@gotwarlost Hello from 2023. Can we please get this merged?

pzrq commented 12 months ago

Assuming this repo remains confusingly not archived, from npm's instanbul:

This module is no longer maintained, try this instead: npm i nyc Visit https://istanbul.js.org/integrations for other alternatives.

StrictlySkyler commented 10 months ago

Looking for a fix on this issue. The nyc command doesn't seem to fix the issue in the babel-plugin-istanbul package, which seems to be the cause of this.

Thanks,