Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow creating new assertions #34

Closed
wants to merge 2 commits into from
Closed

Allow creating new assertions #34

wants to merge 2 commits into from

Conversation

rstacruz
Copy link
Collaborator

@rstacruz rstacruz commented Oct 4, 2015

Just a proof of concept of #32.

@rstacruz
Copy link
Collaborator Author

rstacruz commented Oct 5, 2015

so whats the verdict? :-)

@mjackson
Copy link
Owner

mjackson commented Oct 5, 2015

I think the only thing that makes me a little uncomfortable about this API is that it's not explicit enough.

In jQuery, you have this global jQuery object and if you want to use a plugin you just drop another <script> tag in your page. If we use that pattern, people will end up doing something like:

import expect from 'expect'
import extension from 'expect-extension'

// they won't actually use the "extension" var down here, so linters
// will complain about an unused var

What if we did this instead?

import expect from 'expect'
import extension from 'expect-extension'

// here we actually use the "extension" var
expect.extend(extension)

Also, this way extension authors won't need to put anything on expect.fn. Instead, they can just:

import { assert } from 'expect'

export function toBeAColor(value) {
  assert(
    isColor(value),
    'Expected %s to be a color, but it is not',
    value
  )
}

mjackson added a commit that referenced this pull request Oct 5, 2015
@mjackson mjackson closed this in 03bf0f0 Oct 5, 2015
@rstacruz
Copy link
Collaborator Author

rstacruz commented Oct 6, 2015

oh, perfect.

@rstacruz
Copy link
Collaborator Author

rstacruz commented Oct 6, 2015

Actually, we can go one step further and export assert as this.assert. This way, an expect extension doesn't need to have a direct dependency on expect. What do you think?

@mjackson mjackson deleted the feature/extension branch October 13, 2015 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants