Add script that registers expect as a side-effect

[inancgumus]

I've added expect and assert as side-effects to make mocha auto register expect and assert from CLI automatically and to use them effectively when importing within native modules.

For more information see: #594 and #604

Native Modules Usage

import assert from 'chai/assert'
// Using Assert style
import expect from 'chai/expect'
// Using Expect style
import should from 'chai/should'
// Using Should style

Usage with Mocha

mocha spec.js -r chai/assert
# Using Assert style
mocha spec.js -r chai/expect
# Using Expect style
mocha spec.js -r chai/should
# Using Should style

[lucasfcosta]

Hi @DeeperX, thanks for your PR! 😄
Well, since we've already got this for should I think it makes sense not only to do this for expect, but for assert too. If we're doing this I think we should do it "completely", since it would allow anybody to write their tests cleanly.

This LGTM, but I'd like to hear what the rest of the team thinks about it.

[shvaikalesh]

Hey @DeeperX, that is awesome and fits great with Mocha's approach. Agreed with @lucasfcosta that we should do the same for assert. However, maybe we should be more explicit that we add global variable, like require('chai/global-expect')?

[meeber]

I don't have a strong preference between expect and global-expect. Agreed that an assert version should be added. We should also consider updating the existing should script to register the should global.

[inancgumus]

Thanks guys for your comments. I added assert to the pull request. I also added global.should and global.assert.

I believe that chai/expect is simpler than chai/global-expect which is more verbose although more explicit. And, I believe that anyone wants to use these side effects also wants to make expect and others like should and assert to be globalized. At least, that's what I would want it to behave like.

[meeber]

I like this but I just had another thought: we should document it in the Usage section of the README.

[inancgumus]

@meeber I updated README.md for the usage examples.

[lucasfcosta]

I think this should be named "Native Modules Usage" or something like that. "Usage for ES6" sounds a bit strong and makes it look like its the only way to use it in ES6.

[inancgumus]

OK, I updated it like you suggested:

[lucasfcosta]

This is looking good, except for the title for the ES6 usage.
I think that after that change it would be ready for a merge.

[lucasfcosta]

Awesome job @DeeperX!
Thanks for your PR.
This LGTM.

[keithamus]

Cool, this LGTM! Thanks very much for taking the time to address feedback @DeeperX, and kudos for contributing to chai!

[shvaikalesh]

I am sorry for the late response, but I can't find in README something like

Beware, this exposes a global variable

Furthermore,

import assert from 'chai/assert'
// Using Assert style
import expect from 'chai/expect'
// Using Expect style
import should from 'chai/should'

does not look as fancy as

import {assert, expect, should} from 'chai'

[inancgumus]

@shvaikalesh I can add that warning to the README.

Btw, I don't think that anyone will want to use all of the styles together like this (I guess you just have made it as an example):

import { assert, expect, should } from 'chai'

But, can use it like this:

import { assert } from 'chai'

But, why do you fancy on more verbose side rather than the latter simpler one without the braces? I can also do it with braces if everyone is agree.

[shvaikalesh]

@DeeperX Yop, agreed on "all of the styles together" here.

Comparing

import assert from 'chai/assert'

vs

import { assert } from 'chai'

1. Named exports/imports is ES6 modules way.
2. It does not rely on internal file structure, like import assert from 'chai/assert' does
3. A little bit cleaner (assert is written only once) and shorter (weak argument)

[inancgumus]

@shvaikalesh I see your point as valid reasons. I can add an exposing module so we can use the ES6 modules way.

[shvaikalesh]

@DeeperX Thanks for having this discussion & contributing to Chai 👍 . I am curious what other guys think on this matter.

[shvaikalesh]

Yet another point: importing { util } or some other stuff along with except or assert

[inancgumus]

@shvaikalesh Your welcome. Thank you for your good ideas. Btw, this pull request is about exposing chai testing styles to CLI and native modules. Do you think that util is relevant here? Or should we add it in another request/commit?

[meeber]

@DeeperX Thanks for your continued work on this! There are so many different ways to use Chai. Let's review these and discuss which ones we want to document, and which ones require warnings (due to creating global variables or something). Also I think some of the examples that got pushed with this PR are inadvertently mixing styles.

Vanilla style:

var chai = require('chai');
var assert = chai.assert;    // Creates local variable `assert`
var expect = chai.expect;    // Creates local variable `expect`
var should = chai.should();  // Creates local variable `should` and modifies `Object.prototype`

Vanilla style alternative using ES6 destructuring (really shines when using plugins):

const {assert} = require("chai");  // Creates local variable `assert`
const {expect} = require("chai");  // Creates local variable `expect` 
const {should} = require("chai");  // Creates local variable `should`
should();  // Modifies `Object.prototype`

const {expect, use} = require("chai");  // Creates local variables `expect` and `use`; useful for plugin use

Vanilla style alternative using ES6 destructuring and ES6 modules (really shines when using plugins):

import {assert} from "chai";  // Creates local variable `assert`
import {expect} from "chai";  // Creates local variable `expect`
import {should} from "chai";  // Creates local variable `should`
should();  // Modifies `Object.prototype`

import {expect, use} from "chai";  // Creates local variables `expect` and `use`; useful for plugin use

Side-effect style:

require("chai/assert");  // Creates global variable `assert`
require("chai/expect");  // Creates global variable `expect`
require("chai/should");  // Creates global variable `should` and modifies `Object.prototype`

Side-effect style alternative using ES6 modules:

import "chai/assert";  // Creates global variable `assert`
import "chai/expect";  // Creates global variable `expect`
import "chai/should";  // Creates global variable `should` and modifies `Object.prototype`

Side-effect style alternative using Mocha CLI arguments:

# Creates global variable `assert`:
mocha -r chai/assert

# Creates global variable `expect`:
mocha -r chai/expect

# Creates global variable `should` and modifies `Object.prototype`:
mocha -r chai/should

[shvaikalesh]

I have concerns that it might not be clear to all developers that import {assert} from "chai" and import "chai/assert" are different. I think we should consider documenting "Creates global variable assert" thing or require more verbose chai/register-assert or something like this.

[meeber]

@shvaikalesh Ah yes, laying out all the different styles like this strengthens your earlier argument regarding naming.

[inancgumus]

@shvaikalesh For example, should() is modifying object prototype but this is not named in the should() method like prototypeOverwritingShould(). It's just should() for ease of usage. But, this is documented. So, people know what is this doing. Adjusting the object prototype is a much dangerous thing than exposing a global for just test only even everyone is aware of this already.

Think about this scenario, even we're now exposing it globally, anyone who wants to use it in her code will expose it globally, mostly.

import {assert} from "chai";  // exposed it globally
var assert = chai.assert;     // exposed it globally

Will use assert for the testing in the whole document from a global variable. I don't think that anyone is going to use assert for each local method testing. Like this:

describe('something', () => {
  it('does something', () => {
    var assert = chai.assert;
    assert.equal(true, true);
  })

  it('does something lese', () => {
    var assert = chai.assert;
    assert.equal(false, false);
  }) 
})

It's already has a global usage.

I'm +1 for ES6 native module support through an exposed module. But, I'm still undecisive for chai/register-assert instead of just: chai/assert.

I'll implement it like that if everyone is agree but will not be happy using it :)

[shvaikalesh]

@meeber Thanks for laying it all out. I think I am not confused anymore 😄.

@DeeperX

I think that module should either export something or set as global variable/Object.prototype property. Not both. Otherwise, it seems confusing to me. We should distinguish Mocha API from regular one.

import { assert } from "chai/register-assert"; // nope
import "chai/register-assert"; // yep

[keithamus]

import { assert } from "chai/register-assert"; // nope
import "chai/register-assert"; // yep

I endorse this. This is the way it should be documented. With an addition of

import { assert } from 'chai'; // yup
import { assert } from 'chai/register-assert'; // nope

[meeber]

@DeeperX I don't think anyone is suggesting that developers keep redeclaring a variable like assert local to each it or describe, but rather at the top of each test module file. It may have been more appropriate in my examples to distinguish module-scope versus global-scope.

[inancgumus]

OK, guys I hear you all.

So, to summarize, we will be doing like this, right? Please correct the examples if I missed something:

Native Modules Usage (globally)

import 'chai/register-assert'
// Using Assert style
import 'chai/register-expect'
// Using Expect style
import 'chai/register-should'
// Using Should style

Native Modules Usage (local import only)

import { assert } from 'chai';
// Using Assert style
import { expect } from 'chai'
// Using Expect style
import { should } from 'chai'
// Using Should style

Usage with Mocha

mocha spec.js -r chai/register-assert
# Using Assert style
mocha spec.js -r chai/register-expect
# Using Expect style
mocha spec.js -r chai/register-should
# Using Should style

[meeber]

I don't think a decision has been made yet.

I agree with @shvaikalesh that the register scripts should only register a global/modify Object.prototype, not also export.

However, I'm still undecided on naming for the side-effect style:

import "chai/assert";  // Shorter but less clear that there's a global side-effect
import "chai/register-assert";  // Longer but more clear that there's a global side-effect

# Shorter and obvious based on context that there's a global side-effect
mocha -r chai/assert

# Longer and kind of redundant
mocha -r chai/register-assert

In other words, I prefer @shvaikalesh's style when used inside a JavaScript file, but I prefer @DeeperX's style when used as a Mocha require. I think I'm actually leaning toward @DeeperX's style as my preference because I think the side-effect style has a much more common use case in the Mocha context (and we could tailor our documentation toward that end). But both sides make good points. Hmm. Let's get some more opinions: @lucasfcosta @vieiralucas ?

[lucasfcosta]

Hmm, I have mixed feelings about it.
At first I'd say we should go with chai/assert only, but now that I've thought a little bit about it I think this use case is kind of uncommon and therefore what it does should be really explicit to the ones using it, in order to avoid confusing other users.
I'd go with the explicit option chai/register-<name> looks better to me.

[meeber]

It looks like the chai/register-<name> syntax has the most support from the team. Let's go with that.

@DeeperX Couple comments regarding the draft you posted above:

• I think we should document the pre-native module versions of these too (e.g., const { assert } = require('chai'); and require('chai/register-assert');).
• I think we should put each example's comment either before or on the same line as the example, instead of afterward.

[inancgumus]

So, going one turn again. Please check this examples and please let me know if it's OK:

---

Preferably, you should only use only one assertion style in your tests: assert, expect, or should.

Pre-Native Modules Usage (registers the chai testing style globally)

require('chai/register-assert')  // Using Assert style
require('chai/register-expect')  // Using Expect style
require('chai/register-should')  // Using Should style

Pre-Native Modules Usage (as local variables)

const { assert } = require("chai")  // Using Assert style
const { expect } = require("chai")  // Using Expect style
const { should } = require("chai")  // Using Should style
should()  // Modifies `Object.prototype`

const { expect, use } = require("chai")  // Creates local variables `expect` and `use`; useful for plugin use

Native Modules Usage (registers the chai testing style globally)

import 'chai/register-assert'  // Using Assert style
import 'chai/register-expect'  // Using Expect style
import 'chai/register-should'  // Using Should style

Native Modules Usage (local import only)

import { assert } from 'chai'  // Using Assert style
import { expect } from 'chai'  // Using Expect style
import { should } from 'chai'  // Using Should style
should()  // Modifies `Object.prototype`

Usage with Mocha

mocha spec.js -r chai/register-assert  # Using Assert style
mocha spec.js -r chai/register-expect  # Using Expect style
mocha spec.js -r chai/register-should  # Using Should style

[meeber]

@DeeperX Again, thank you for your perseverance on this. It's greatly appreciated by the team :D

Thoughts:

• The comments inside of Pre-Native Modules Usage (as local variables) are different than the comments inside all of the other usages.
• Native Modules Usage (local import only) needs a should() after the import.
• Just to be clear, the actual PR that makes these changes should also get rid of the module.exports = part of the three newly-renamed register files, so that they can only be used in the global side-effect capacity.

[inancgumus]

@meeber Thanks for your review, I really appericiate it. No problem btw, let's do it as neat as we can :)

I updated my previous post according to your review. And, I'll remove module.exports = from the new register files.

[inancgumus]

Hi, I added new commits to my branch here, but it seems like it didn't appear here. Should I create a new pull request?