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

Rule proposal: Forbid imports with explicit side-effects #529

Closed
jfmengels opened this issue Aug 28, 2016 · 6 comments
Closed

Rule proposal: Forbid imports with explicit side-effects #529

jfmengels opened this issue Aug 28, 2016 · 6 comments

Comments

@jfmengels
Copy link
Collaborator

jfmengels commented Aug 28, 2016

IMO, modules that have side-effect just from requiring them are really bad. I would like to see a rule that forbids importing/requiring modules that are not assigned, which implicitly means there is a side-effect or simply not used and therefore useless.

Invalid

import 'dosideeffect';
import './dosideeffect';
require('./dosideeffect');

Valid

import foo from 'bar';

// Explicit effect
import doEffect from './dosideeffect';
doEffect();

As far as rule name goes: no-unnassigned-import, no-side-effect-import, ...? I'm leaning toward the former at the moment, though it doesn't capture the rationale of it.

@benmosher
Copy link
Member

What if the rule instead enforced that modules not have side effects? (or only side effects?)

The import-er may not have the option to refactor to an explicit effect, and if he/she does, then a no-side-effects rule would flag the import-ed module instead of the import-ing module.

@jfmengels
Copy link
Collaborator Author

I could see the value for both, but I agree that a no-side-effects rule would be better. Except I don't see how you'd go and do that... 😕

@jfmengels
Copy link
Collaborator Author

I'd like to work on this soon-ish, especially as it's pretty easy to implement.

I still don't see how you can enforce having modules not create side-effects. That'd be awesome if it was possible though...

@benmosher
Copy link
Member

Works for me. Name is still not obvious, but that doesn't need to hamper development of the rule, obviously.

I thought I had said this in this issue before, but no-side-effects could warn on the first line of any module with imports, but no exports. Not sure how to handle the entry point of applications in that case, though. And again. no need to hold up the simple version of this rule (flagging imports without names).

@jfmengels
Copy link
Collaborator Author

no-side-effects could warn on the first line of any module with imports, but no exports

Sure, but that does not really prevent them from having side-effects too.
Think require('should');, which both exports something and has a side-effect (sometimes you'll use the exported value, sometimes you won't). Since that would not work for external libs, that would not help the case I'm trying to solve here.

@benmosher
Copy link
Member

Yeah, fair enough. I think your original proposal makes the most sense, at least right now.

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