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

New rule: No arrow function expression as test callback #24

Closed
mitchlloyd opened this issue Mar 1, 2016 · 5 comments
Closed

New rule: No arrow function expression as test callback #24

mitchlloyd opened this issue Mar 1, 2016 · 5 comments
Assignees
Milestone

Comments

@mitchlloyd
Copy link
Contributor

As arrow functions become more commonly used, developers may use them to create QUnit test callbacks. This changes the expected test context to something else (probably to window). Some testing setups have specialized modules that use this inside beforeEach to manipulate the context, so changing it can be problematic.

Example that should warn:

test('description', (assert) => {
});

Example that is ok:

test('description', function(assert) {
});

I haven't thought too much about implementation yet, but it seems fairly simple to look for the test callback and then ensure that an arrowFunctionExpression is not being used.

It might also make sense to apply this rule to the beforeEach and afterEach callbacks in module setup.

@platinumazure
Copy link
Collaborator

Excellent call, agree that it should be on test and module callbacks, consider this accepted (until I can get to a desktop and apply the label).

Thoughts on rule name? I'm thinking no-arrow-tests, but maybe you can do better, I'm feeling meh. Feel free to submit a PR

@platinumazure
Copy link
Collaborator

I'll start work on this.

@mitchlloyd Do you think there should be an option to tolerate arrow functions if they don't refer to this? In theory, that shouldn't cause problems. (I would still want the default option to be outright forbidding arrow functions as test callbacks, though.)

@platinumazure platinumazure self-assigned this Mar 12, 2016
@mitchlloyd
Copy link
Contributor Author

I don't have any interest in allowing people to use both => and function for defining test callbacks. I just don't see any advantage to allowing the => style.

@platinumazure
Copy link
Collaborator

@mitchlloyd I'm with you on that-- I'm just trying to figure out if there might be others who would want the linter to only flag it when someone is actually relying on this in an arrow function.

It's easy to add this later if someone wants it, so I guess for now I'll go with no arrow functions period. In that case, I'm pretty close to having a branch ready.

@platinumazure
Copy link
Collaborator

I've put up a new branch. Could you take a look at the documentation in that branch to make sure I haven't misrepresented anything (especially about arrow functions, which I don't regularly use)? Thanks!

wridgeu added a commit to wridgeu/ui5-msg-box-sequencer that referenced this issue Jul 31, 2023

Verified

This commit was signed with the committer’s verified signature.
MichaelDeBoey Michaël De Boey
arrow functions and their context binding are important

note: qunitjs/eslint-plugin-qunit/issues/24#issue-137707265

also in case this.stub ever fails, simply use sinon directly `sinon.stub`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants