Improvement for no-relative-imports rule: allow parent imports (like we have allow-siblings)
sharikovvladislav opened this issue · 5 comments
Feature request
Is your feature request related to a problem? Please describe.
tslint
has rule no-relative-imports
out-of-the-box. This rule has allow-siblings
option. So we can write paths in two ways:
- no relative paths at all without option, but using this rule
./*
relative paths are allowed with option
In our team we have an arrangement, which allows us to use ../*
paths. Only 1 step above, more (like ../../*
) is restricted.
This rule + option is not flexible enough for this.
Describe the solution you'd like
I suggest to add one more option, which will specify amount of steps we can go above. Or I think it would be better, to add allow-parent
option. It is more strict and applies better to general idea of the rule.
Describe alternatives you've considered
I don't have any ideas of alternatives :) I can just write custom rule for our project. But I think it is some feature that can be used not only by our team.
I can create PR if you don't mind adding this to the project.
Honestly speaking while I understand why would you like mentioned options and behavior, I don't think that this is good fit for existing rule and would be better served as independent more configurable rule.
While allowing ../*
contradicts rule name in the same way as ./*
later is more common pattern which enables code splitting and rarely hurts code movements.
In other benefits of separate rule - allow-parent
behavior might be restricted only to few file or import path patterns with configuration that doesn't exists in current rule.
As workaround for you can use import-blacklist
core rule and passing pattern as shown below
{
"rules": {
"import-blacklist": [true, ["/\\.\\./"]]
}
}
import tst from 'test';
import tst from './test';
import tst from '../test';
import tst from '../../test'; // Error
import tst from 'aa/../test'; // Error
import tst = require('aa/bb/test');
import tst = require('../../test'); // Error
import tst = require('aa/../test'); // Error
const tst = require('aa/bb/test');
const tst = require('../../test'); // Error
const tst = require('aa/../test'); // Error
Not the best error message This import is blacklisted by /\/\.\.\//
, but can be negotiated to the team
If by some chance you use windows path style - than RegExp in config will be a bit cryptic "import-blacklist": [true, "", ["[/\\\\\\\\]\\.\\.[/\\\\\\\\]"]]
more configurable rule.
in what ways it should be more configurable? depth?
In other benefits of separate rule - allow-parent behavior might be restricted only to few file or import path patterns with configuration that doesn't exists in current rule.
I don't understand this point. Could you explain more verbose, please?
which enables code splitting and rarely hurts code movements.
Does ../*
have problems with it? Looks like tools fixing everything allright. Or I don't understand the point again :P
With allow-parent
option current rule matches behavior of import/no-relative-parent-imports
ESLint rule and I doubt that this rule should move further.
Does arrangement in your team allow ../*
imports everywhere or under some conditions?
In my opinion ../*
are more prone to bring errors during move (not all tools will update paths correctly) or especially copying and shouldn't be allowed all across the project. Therefore some rules should be enforced to handle path with references to parent folders.
More configuration may be (and will be more project specific):
- depth
- condition for module specifier (e.g. parent references are allowed only like
from '../../Theme'
orfrom '../style.css'
, but would be disallowed infrom '../connectors'
) - , condition for file location (only files located in
sub-component
folder can have parent imports, but inconfig
folder they are disallowed)
This kind of configuration is out of scope for no-relative-imports
rule.
I will leave this issue open so other maintainers can weight in on your proposal, however I'm not sure that no-relative-imports
rule should support any parent references given available workaround with import-blacklist
rule form TSLint.
given available workaround with
import-blacklist
Its ok. Feel free to close the issue, If you don't think this is applicable for tslint in general.
Also, I can create my custom rule based on any tslint rule and use it. We don't have this problem now. Thank you for import-blacklist