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

qunit/resolve-async doesn't recognise .call/.apply #68

Closed
edg2s opened this issue Jun 5, 2018 · 3 comments
Closed

qunit/resolve-async doesn't recognise .call/.apply #68

edg2s opened this issue Jun 5, 2018 · 3 comments

Comments

@edg2s
Copy link
Contributor

edg2s commented Jun 5, 2018

The following example fails linting, despite done being called in three different ways

beforeEach: function ( assert ) {
	var done = assert.async();
	done.call( this );
	done.apply( this );
	$.Deferred().promise().resolve().then( done );
}

the third case being what we use here: https://github.com/wikimedia/VisualEditor/blob/master/tests/ce/ve.ce.Surface.test.js#L10

@platinumazure
Copy link
Owner

Hi @edg2s, thanks for the issue.

I would regard the first two (.call, .apply) as bugs, since the intention was to catch direct calls and those are equivalent to the simple function call.

The last, I would say is a rule enhancement. I'm not sure whether I want to allow all function calls with the callback being passed (either by option or by default), or if instead we could use AST selectors to ensure that only certain function calls with the callback would "count" (e.g., must be a call to .then()). What are your thoughts?

@edg2s
Copy link
Contributor Author

edg2s commented Feb 19, 2019

I think whitelisting the caller to then would be acceptable. That would match ES6 & jQuery promises.

@platinumazure
Copy link
Owner

The .call/.apply issue will be resolved with the upcoming 5.0.0 release.

I'll create a separate issue for the enhancement request of tracking if the callback is passed to specific methods such as .then.

@platinumazure platinumazure changed the title qunit/resolve-async doesn't recognise unconventional calls to 'done' qunit/resolve-async doesn't recognise .call/.apply May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants