Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add ability to ban global functions #327

Closed
matthewoates opened this issue Feb 22, 2015 · 10 comments
Closed

Add ability to ban global functions #327

matthewoates opened this issue Feb 22, 2015 · 10 comments

Comments

@matthewoates
Copy link

ie: alert

@adidahiya
Copy link
Contributor

This would likely be an extension of the ban rule.

@adidahiya adidahiya changed the title cannot ban global functions Add ability to ban global functions Dec 1, 2015
@mmmichl
Copy link

mmmichl commented Feb 12, 2016

+1 subscribing

@thasner
Copy link

thasner commented Mar 23, 2016

👍 , specifically banning window.alert

@donaldpipowitch
Copy link
Contributor

More use cases: fit and fdescribe in tests.

@soniro
Copy link
Contributor

soniro commented Jul 12, 2016

Do you prefer extending the ban rule or creating a separate rule?
We've just implemented a "global-ban" rule (because we want to ban fit, fdescribe, xit and xdescribe) in our project which I'd love to share.

IMHO it would be most intuitive to extend the existing ban rule, but this could be confusing in the configuration. How would a config then look like? Is a global function configuration just a string? Or an array of one string (which is described in the documentation as an object)?

"ban": [true, "someGlobalFunction", ["console", "log"], ["someObject", "someFunction"]]

So for a clear configuration a separate rule could make sense.

If you prefer a separate rule: Are you fine with the name "global-ban"? It feels there could be a better name.

I will send a pull request tonight or tomorrow.

@donaldpipowitch
Copy link
Contributor

I think extending the ban rule would be nicer, but I would be fine with a new role called global-ban.

@soniro
Copy link
Contributor

soniro commented Jul 12, 2016

And how would you configure it in the ban rule? As a one item array (which would follow the syntax for the other bans) or as single strings (which may be a bit confusing)?

My first implementation uses a single item array:
"ban": [true, ["someGlobalFunction"], ["console", "log"], ["someObject", "someFunction"]]
For me this feels more consistent.

@donaldpipowitch
Copy link
Contributor

Oh, I don't know :) Maybe others have an opinion about this.

@ygalustov
Copy link

soniro commented 5 days ago
"ban": [true, "someGlobalFunction", ["console", "log"], ["someObject", "someFunction"]]

Looks good, or as an object it could be some reserved word (e.g. global) or null/undefined value

"ban": [true, [null, "someGlobalFunction"], ["console", "log"], ["someObject", "someFunction"]]

I'm looking for ways to ban parseInt and parseFloat.

@soniro
Copy link
Contributor

soniro commented Jul 17, 2016

Me personally, I'm not a huge fan of using null/undefined keywords in such cases.
I'd prefer a solution without writing magic keywords.

I just submitted a pull request. Please have a look at it. If you don't like the suggested syntax, we can again discuss it and change the implementation. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants