standard-things / esm

Tomorrow's ECMAScript modules today!
Other
5.26k stars 147 forks source link

issue with unused imports (and libraries that monkey w/ require caches?) #798

Open boneskull opened 5 years ago

boneskull commented 5 years ago

I've adapted the "proxyquire" scenario from the tests to show an issue. Below, I've added a second assertion, and a third file, c.js. The assertions made below, using CJS, pass (to prove it works as intended):

// index.js
const assert = require("assert")
const proxyquire = require("proxyquire")

// `path` and `b.js` are stubbed ONLY in a.js
const a = proxyquire.load("./a.js", {
    path: {
      extname: () => "c"
    },
    "./b.js": "b"
  })

assert.strictEqual(a(), "abc")

// `path` and `b.js` are now stubbed when loaded by ANY module
const b = new String("b") // this is hideous, but what can you do?
b["@global"] = true
const c = proxyquire.load("./c.js", {
  path: {
    extname: () => "c",
    "@global": true
  },
  "./b.js": b
})

assert.strictEqual(c(), "abc")
// a.js
const b = require("./b.js")
const {extname} = require("path")

module.exports = () => "a" + b + extname("")
// b.js
// Empty module.
// c.js
const a = require("./a.js")
const {extname} = require("path") // UNUSED

module.exports = () => a()

Now, if we write these using ES modules via node -r esm:

// index.js
import assert from "assert"
import proxyquire from "proxyquire"

const a = proxyquire
  .load("./a.js", {
    path: {
      extname: () => "c"
    },
    "./b.js": "b"
  })

assert.strictEqual(a.default(), "abc")

const b = new String("b")
b["@global"] = true
const c = proxyquire
  .load("./c.js", {
    path: {
      extname: () => "c",
      "@global": true
    },
    "./b.js": b
  })

assert.strictEqual(c.default(), "abc")
// a.js
import b from "./b.js"
import { extname } from "path"

export default () => "a" + b + extname("")
// b.js
// Empty module.
// c.js
import a from "./a.js"
import { extname } from "path" // UNUSED

export default () => a()

The result:

AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 'ab'
+ 'abc'
    at Object.<anonymous> (/Users/boneskull/projects/standard-things/esm/test/fixture/scenario/proxyquire/index.js:25:8)
    at Generator.next (<anonymous>)

If we remove that unused import in c.js:

// c.js
import a from "./a.js"

export default () => a()

the assertion passes.

I don't know what the problem is, but that's why I'm making this issue.

I can replicate this behavior in both rewiremock and proxyquire. I'll probably send a PR to add some rewiremock tests.

boneskull commented 5 years ago

Note:

theKashey commented 5 years ago
  1. Don't use @global with proxyquire. Just don't. It does not play well.
  2. Use require('proxyquire').noPreserveCache()

What's happening:

  1. Proxyquire uses require.extensions[extension] to hook into module loading.
  2. This method is "below" cache
  3. It has check that parentModule === testModule
  4. If check fail - the original file would be loaded and stored in cache.
  5. When the right file would require the file - it would be returned from a cache, in a original form, or in mocked form, depending on the require order.

There the problem is probably bound to tree shaking - unused import was not required in esm, which, however, is required by a fact (just tested it).

I don't know what the problem is, but that's why I'm commenting on this issue.

boneskull commented 5 years ago

@theKashey you may or may not wish to comment on #799