Istanbul: Support for defaults when destructuring parameters

[NotTheUsual]

Hey, Node 6 just shipped with a whole bunch of extra ES6 features, including defaults for destructured parameters, e.g.

function destructure({test = 'not set'}) {
  return `value of "test" is: ${test}`;
}

Mocha tests understand this without problem, but running the same tests through Istanbul (1.0.0-alpha.2) fails: the default is ignored, and any unset values come through as undefined. I appreciate that this may be beyond your control (i.e. the responsibility of a dependency), but figured it was worth flagging.

---

The failing test, for reference:

it('can use a default value for the test', () => {
  expect(index.destructure({})).to.equal('value of "test" is: not set');
});

The Mocha run returns value of "test" is: not set, the Istanbul run returns value of "test" is: undefined

[tomyf]

It also fails for Istanbul 0.4.3 as well. As far as I tried, it is working for default values when not destructuring, but it is not working as @NotTheUsual said for destructuring with default values.

[holm]

This would be great to have fixed. We were looking to get rid of Babel when upgrading to Node 6, but since Istanbul causes default values to not work with destructuring, we have to keep Babel in there.

[tswaters]

This looks like a bug in escodegen... the AST includes the destructured parameter when it hits that point, and when it comes out, the assignments in destructured objects are lost. I've created an issue in their queue for this: https://github.com/estools/escodegen/issues/296

[gotwarlost]

istanbul@next uses babel under the covers for instrumentation. That one should work for this use case. Can you try using this version?

[tomyf]

Testcase :

const tmp = ({test = 'not set'} = {}) => test
// exports...

const tmp = require('../src/tmp'); // function defined above
const {assert} = require('chai');

describe('tmp', () => {
    it('should return the correct value', () => {
        assert.strictEqual(tmp({test: 'AA'}), 'AA');
    });
    it('should return the default value when test is not defined', () => {
        assert.strictEqual(tmp({}), 'not set');
    });
    it('should return the default value when no arguments are given', () => {
        assert.strictEqual(tmp(), 'not set');
    });
});

• Fails with [email protected].
• Works with istanbul@[email protected] (or istanbul@next as you said) with 100% of coverage !

:+1:

[DavidGDD]

@tswaters has opened a pull-request (https://github.com/estools/escodegen/pull/297) to solve this... it seems a problem from escodegen.

We will expect that Istanbul update the dependency as soon as posible escodegen has released the merge.

Until then I'm using isparta with babel and es2015 preset, but i'm trying to avoid babel (and all its dependencies). some one knows an alternative?

[tswaters]

This may not be ideal, but you could always manually patch the escodegen inside your node_modules with the changes from my PR. It should be noted the one in npm doesn't have this commit so you'll need to apply that as well. Seems like istanbul@next fixes this though so it might be easiest to switch your dep to that.

[DavidGDD]

Escodegen has generated a new version (1.8.1) which corrects this issue!

[webliving]

@tomyf i used [email protected] too, why not ? 😄